Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Test for -c option check if port is changed #680

Merged
merged 1 commit into from
Jan 15, 2024
Merged

Test for -c option check if port is changed #680

merged 1 commit into from
Jan 15, 2024

Conversation

ArtiomDivak
Copy link
Contributor

Added an option to add open ports for spawned containers. And also added a test to check if conf file change with -c option will change the ports for agent and ctrl.

Related-to: #668

@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 2 times, most recently from ec4c7a9 to bcd110b Compare January 8, 2024 11:13
mwperina
mwperina requested changes Jan 8, 2024
View reviewed changes
tests/bluechi_test/fixtures.py Outdated Show resolved Hide resolved
tests/tests/tier0/bluechi-change-port-with-c-cmd-option/config-files/agent_port_8421.conf Outdated Show resolved Hide resolved
tests/tests/tier0/bluechi-change-port-with-c-cmd-option/config-files/ctrl_port_8421.conf Outdated Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Outdated Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Outdated Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Outdated Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Show resolved Hide resolved
...ts/tier0/bluechi-change-port-with-c-cmd-option/test_bluechi_change_port_with_c_cmd_option.py Show resolved Hide resolved
@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 2 times, most recently from 4805b3d to 7efc324 Compare January 9, 2024 11:43
@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 5 times, most recently from 12a147e to 832681e Compare January 10, 2024 07:46
@mwperina
Copy link
Member

Adding my thoughts how the test should look like:

  1. Initial setup - controller uses standard port and an agent is connected to it
  2. Restart controller with config file containing different port using command line option
  3. Check that
    • controller started successfully
    • controller is listening on a different port
    • agent is not connected to the controller
  4. Restart agent with config file containing different port using command line option
  5. Check that
    • agent started successfully
    • agent is connected to the controller

@mwperina mwperina mentioned this pull request Jan 10, 2024
Merged
@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 2 times, most recently from 37529c3 to 81e49d4 Compare January 10, 2024 10:33
@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 4 times, most recently from de0f947 to 7627413 Compare January 10, 2024 13:49
mwperina
mwperina approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

LGTM

@mwperina
Copy link
Member

/packit retest-failed

@ArtiomDivak ArtiomDivak force-pushed the issue-668-2 branch 4 times, most recently from c857275 to f486a40 Compare January 11, 2024 10:31
mwperina
mwperina approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

Checking for controller/agent process in lsof output is better than original solution with checking lsof return code 👍

@ArtiomDivak
Test for -c option check if port is change
efd1943 
Added an option to add open ports for spawned containers.
And also added a test to check if conf file change with
-c option will change the ports for agent and ctrl.

Related-to: eclipse-bluechi#668
Signed-off-by: Artiom Divak <adivak@redhat.com>
engelmi
engelmi approved these changes Jan 15, 2024
View reviewed changes
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.

LGTM

@engelmi engelmi merged commit ed15bd5 into eclipse-bluechi:main Jan 15, 2024
18 checks passed
@engelmi engelmi mentioned this pull request Jan 23, 2024
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@engelmi engelmi engelmi approved these changes

@mwperina mwperina mwperina approved these changes

@alexlarsson alexlarsson Awaiting requested review from alexlarsson alexlarsson is a code owner

@mkemel mkemel Awaiting requested review from mkemel mkemel is a code owner

@rhatdan rhatdan Awaiting requested review from rhatdan rhatdan is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArtiomDivak @mwperina @engelmi