Skip to content

Fix(tests): start local agent by default #1065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dracher
Copy link
Contributor

@dracher dracher commented Apr 2, 2025

update all test cases(which requires 2 agents) to use node-local as second agent remove redundant agent configs and use the controller machine for local agent operations simplify test setups by removing unnecessary agent configuration

current progress:

  • monitor-wildcard-unit-changes
  • proxy-service-start
  • bluechi-reset-failed
  • monitor-multiple-nodes-and-units
  • monitor-system-status 3 agents
  • bluechi-nodes-statuses
  • bluechi-is-oneline-system
  • bluechi-reset-failed
  • bluechi-reset-failed-node
  • proxy-service-stop-target
  • proxy-service-stop-requesting
  • proxy-service-stop-bluechi-dep
  • bluechi-list-units-on-all-nodes
  • monitor-wildcard-node-reconnect
  • bluechi-is-online-system-monitor
  • monitor-multiple-nodes-and-units
  • proxy-service-fails-on-execstart
  • proxy-service-fails-on-typo-in-file
  • bluechi-agent-invalid-port-configuration
  • proxy-service-multiple-services-one-node
  • service-fails-on-non-existent-service
  • service-stop-requesting-with-unneeded
  • multiple-services-multiple-nodes
  • propagate-target-service-failure
  • socket-activation-parallel

Fixes: #1042

@dracher dracher force-pushed the VROOM-26373 branch 2 times, most recently from 4b1ab93 to 979a98f Compare April 9, 2025 12:35
@coveralls
Copy link

coveralls commented Apr 9, 2025

Coverage Status

coverage: 82.278% (+0.04%) from 82.234%
when pulling 1baff88 on dracher:VROOM-26373
into 30cf42a on eclipse-bluechi:main.

@dracher dracher marked this pull request as ready for review April 10, 2025 02:54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, shouldn't we remove this test completely as we test local UDS connection by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 +1

@@ -253,6 +261,9 @@ def setup(
if self.bluechi_controller_config is None:
raise Exception("Bluechi Controller configuration not set")

if self.bluechi_local_agent_config is not None:
self.bluechi_controller_config.allowed_node_names.append("node-local")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, instead of appending node-local to allowed_node_names here wouldn't it be cleaner to do that inside BluechiController Machine class?

class BluechiControllerMachine(BluechiMachine):
    def __init__(
        self,
        name: str,
        client: Client,
        ctrl_config: BluechiControllerConfig,
        agent_config: BluechiAgentConfig = None,
    ) -> None:
        super().__init__(name, client)

        self.bluechictl = BluechiCtl(client)

        self.config = ctrl_config

        self.config_agent = agent_config
        if self.config_agent is not None:
            self.config.allowed_node_names.append(self.config_agent.node_name)
            self.create_file(
                self.config_agent.get_confd_dir(),
                self.config_agent.file_name,
                self.config_agent.serialize(),
            )

        # add confd file to container
        self.create_file(
            self.config.get_confd_dir(), self.config.file_name, self.config.serialize()
        )

        self.copy_machine_lib()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current implementation, it can't append the node-ctrl name inside BluechiController Machine class, it will be overwrite by it

update all test cases(which requires 2 agents) to use node-local as second agent
remove redundant agent configs and use the controller machine for local agent operations
simplify test setups by removing unnecessary agent configuration

current progress:

- [x] monitor-wildcard-unit-changes
- [x] proxy-service-start
- [x] bluechi-reset-failed
- [x] monitor-multiple-nodes-and-units
- [x] monitor-system-status 3 agents
- [x] bluechi-nodes-statuses
- [x] bluechi-is-oneline-system
- [x] bluechi-reset-failed
- [x] bluechi-reset-failed-node
- [x] proxy-service-stop-target
- [x] proxy-service-stop-requesting
- [x] proxy-service-stop-bluechi-dep
- [x] bluechi-list-units-on-all-nodes
- [x] monitor-wildcard-node-reconnect
- [x] bluechi-is-online-system-monitor
- [x] monitor-multiple-nodes-and-units
- [x] proxy-service-fails-on-execstart
- [x] proxy-service-fails-on-typo-in-file
- [x] proxy-service-multiple-services-one-node
- [x] service-fails-on-non-existent-service
- [x] service-stop-requesting-with-unneeded
- [x] multiple-services-multiple-nodes
- [x] propagate-target-service-failure
- [x] socket-activation-parallel

- use global constant for node-ctrl name

Fixes: eclipse-bluechi#1042
Copy link
Member

@engelmi engelmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment, but otherwise LGTM

@@ -63,6 +63,9 @@ def test_agent_invalid_configuration(
bluechi_node_default_config: BluechiAgentConfig,
bluechi_ctrl_default_config: BluechiControllerConfig,
):
# Disable default local agent on controller node using UDS,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but would be great to fix it here:
The name of the function should reflect the file name - so test_agent_invalid_configuration should be renamed to test_bluechi_agent_connect_via_controller_address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimize number of nodes in integration tests by having bluechi-agent running on controller node
4 participants