QE: Fix and improve the server hostname rename test #7700

nodeg · 2023-10-17T15:14:48Z

What does this PR change?

This

fixes the server hostname rename test
added some basic hostname and certificate check after each rename
moves the step When(/^I apply highstate on "([^"]*)"$/) do |host| to salt_steps.rb since it belongs there
reverts the update_ca method to its original form because we only need it on the controller

Fixed test

dominik-ctl:~/spacewalk/testsuite # cucumber features/secondary/srv_rename_hostname.feature
(...)
22 scenarios (10 skipped, 12 passed)
45 steps (10 skipped, 35 passed)
5m39.206s
dominik-ctl:~/spacewalk/testsuite #

Improvements

I implemented some basic certificate and hostname checks which did work:

(...)
  Scenario: Check all new server certificates on the minions         # features/secondary/srv_rename_hostname.feature:131
      This scenario ran at: 2023-10-19 14:42:12 +0200
    When I check all certificates after renaming the server hostname # features/step_definitions/command_steps.rb:1626
      Server certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      proxy certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      sle_minion certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      ssh_minion certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      This scenario took: 1 seconds

24 scenarios (10 skipped, 14 passed)
47 steps (10 skipped, 37 passed)
5m42.736s

GUI diff

No difference.

DONE

Documentation

No documentation needed: only internal and user invisible changes
DONE

Test coverage

Cucumber tests were added
DONE

Links

https://gitlab.suse.de/galaxy/infrastructure/-/merge_requests/818
Manager 4.3: https://github.com/SUSE/spacewalk/pull/22927
DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

Re-run test "changelog_test"
Re-run test "backend_unittests_pgsql"
Re-run test "java_pgsql_tests"
Re-run test "schema_migration_test_pgsql"
Re-run test "susemanager_unittests"
Re-run test "javascript_lint"
Re-run test "spacecmd_unittests"

srbarrios · 2023-10-18T09:04:20Z

testsuite/features/secondary/srv_rename_hostname.feature

+@sle_minion
+  Scenario: Apply high state on the SUSE Minion to populate new server CA
+    When I apply highstate on "sle_minion"
+
+@ssh_minion
+  Scenario: Apply high state on the SUSE SSH Minion to populate new server CA
+    When I apply highstate on "ssh_minion"
+
+@rhlike_minion
+  Scenario: Apply high state on the Red Hat-like Minion to populate new server CA
+    When I apply highstate on "rhlike_minion"
+
+@deblike_minion
+  Scenario: Apply high state on the Debian-like Minion to populate new server CA
+    When I apply highstate on "deblike_minion"
+
+@buildhost
+  Scenario: Apply high state on the build host to populate new server CA
+    When I apply highstate on "build_host"
+
+@virthost_kvm
+  Scenario: Apply high state on the virthost to populate new server CA
+    When I apply highstate on "kvm_server"
+
+@pxeboot_minion
+  Scenario: Apply high state on the PXE boot minion to populate new server CA
+    When I apply highstate on "pxeboot_minion"


I would use SSM to apply high state to all the registered systems at the same time.
That way if we add any other system in our CI test suite later, we don't need to go here and maintain this part.

Good idea. I still have issues with high state for the proxy when doing it with Salt:

@proxy Scenario: Apply high state on the proxy to populate new server CA # features/secondary/srv_rename_hostname.feature:67 This scenario ran at: 2023-10-18 09:37:30 +0200 Given the Salt master can reach "proxy" # features/step_definitions/salt_steps.rb:11 It took 1 seconds to contact the minion And I apply highstate on "proxy" # features/step_definitions/command_steps.rb:241 Timeout after 250 seconds (repeat_until_timeout), last result was: bash: dominik-pxy.mgr.suse.de: command not found (RuntimeError) ./features/support/commonlib.rb:117:in `block in repeat_until_timeout' ./features/support/commonlib.rb:100:in `repeat_until_timeout' ./features/support/lavanda.rb:193:in `run_until_ok' ./features/step_definitions/command_steps.rb:248:in `/^I apply highstate on "([^"]*)"$/' features/secondary/srv_rename_hostname.feature:69:in `I apply highstate on "proxy"'

which uses

When(/^I apply highstate on "([^"]*)"$/) do |host| system_name = get_system_name(host) if host.include? 'ssh_minion' cmd = 'mgr-salt-ssh' elsif host.include? 'minion' or host.include? 'build' cmd = 'salt' end get_target('server').run_until_ok("#{cmd} #{system_name} state.highstate") end

And running salt dominik-pxy.mgr.suse.de state.highstatemanually from the server works.

After wondering about SSM, I think we do not yet have a step to select all systems from the systems list and add them to the SSM.

Here a new proposal scenario that will apply high-state in all registered systems:

Scenario: Apply high-state to all systems registered When I follow the left menu "System > System List > All" And I click on "Select All" And I follow the left menu "Systems > System Set Manager > Overview" And I click on "Apply" And I click on "Apply Highstate" And I click on the clear SSM button

Thanks for the suggestion. I will do some test run with this code.

Scenario: Apply high-state to all systems registered # features/secondary/srv_rename_hostname.feature:34 This scenario ran at: 2023-10-20 16:35:03 +0200 When I follow the left menu "System > System List > All" # features/step_definitions/navigation_steps.rb:363 WARN: Step ends with an ajax transition not finished, let's wait a bit! And I click on "Select All" # features/step_definitions/navigation_steps.rb:282 And I follow the left menu "Systems > System Set Manager" # features/step_definitions/navigation_steps.rb:363 WARN: Step ends with an ajax transition not finished, let's wait a bit! And I click on "Apply" # features/step_definitions/navigation_steps.rb:282 Unable to find button "Apply" that is not disabled (Capybara::ElementNotFound) ./features/support/commonlib.rb:146:in `click_button_and_wait' ./features/step_definitions/navigation_steps.rb:283:in `/^I click on "([^"]*)"$/' features/secondary/srv_rename_hostname.feature:38:in `I click on "Apply"' And I click on "Apply Highstate" # features/step_definitions/navigation_steps.rb:282 Then I should see a "Applying the highstate has been scheduled" text # features/step_definitions/navigation_steps.rb:599 And I click on the clear SSM button # features/step_definitions/navigation_steps.rb:799 => /var/log/rhn/rhn_web_ui.log

It seems no systems were selected before. This is the screen when no systems are currently selected:

testsuite/features/secondary/srv_rename_hostname.feature

testsuite/features/step_definitions/command_steps.rb

nodeg · 2023-10-19T12:48:06Z

I implemented some basic certificate and hostname checks which did work:

(...)
  Scenario: Check all new server certificates on the minions         # features/secondary/srv_rename_hostname.feature:131
      This scenario ran at: 2023-10-19 14:42:12 +0200
    When I check all certificates after renaming the server hostname # features/step_definitions/command_steps.rb:1626
      Server certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      proxy certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      sle_minion certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      ssh_minion certificate serial: 0a:cb:26:d4:e6:56:6a:0d:24:a9:52:d1:01:af:ab:a6:ce:36:81:f5
      This scenario took: 1 seconds

24 scenarios (10 skipped, 14 passed)
47 steps (10 skipped, 37 passed)
5m42.736s

nodeg · 2023-10-19T13:29:25Z

I will squash my commits before merging.

Bischoff

LGTM but I could not go into all tiny details.

Nice you have moved the highstate step to salt steps! 👍

Bischoff

LGTM but I could not get into all implied logic, as I did not follow the issue closely.

See nitpick (maybe just a matter of tastes)

Bischoff · 2023-10-20T09:06:25Z

testsuite/features/step_definitions/command_steps.rb

+  hostname, _result = get_target('server').run("hostname")
+  hostname.strip!
+
+  raise "Wrong hostname after changing it. Is: #{hostname}, should be: #{new_hostname}" unless hostname == new_hostname


Maybe such basic checks would deserve a Then step of their own. It would make more explicit / readable that we do them.

Yes, we could do that. However, my main issue with this is, that our hostname rename and then changing the hostname back step, has a hardcoded new hostname variable in it. We do not define this new hostname in the step itself. Which is good, since across all our CIs and PR tests, the prefix of the hostname is different. Those are the reasons why I did not implement a new step of its own to check for the new hostname.

nodeg · 2023-10-20T13:21:03Z

All tests passed.

github-actions · 2023-10-20T13:21:38Z

👋 Hello! Thanks for contributing to our project 😄
☕ Acceptance tests will take same time (aprox. 1h)
👀 Once tests finish, you can check the cucumber report.
See the troubleshooting guide if you need any help.
⚠️ You should not merge if acceptance tests fail to pass
Happy hacking

testsuite/features/secondary/srv_rename_hostname.feature

github-actions · 2023-10-20T13:27:36Z

👋 Hello! Thanks for contributing to our project 😄
☕ Acceptance tests will take same time (aprox. 1h)
👀 Once tests finish, you can check the cucumber report.
See the troubleshooting guide if you need any help.
⚠️ You should not merge if acceptance tests fail to pass
Happy hacking

testsuite/features/secondary/srv_rename_hostname.feature

testsuite/features/step_definitions/command_steps.rb

srbarrios · 2023-10-20T13:47:10Z

testsuite/features/step_definitions/command_steps.rb

+  targets = %w[proxy sle_minion ssh_minion rhlike_minion deblike_minion build_host kvm_server]
+  targets.each do |target|
+    # get all defined minions from the environment variables and check their certificate serial
+    next unless ENV.key? ENV_VAR_BY_HOST[target]


You can either do this or:
next if $node_by_host[target].nil?

But at a higher level, I think you can also do:

$host_by_node.each do |node, _host| minion_cert_serial, result_code = node.run(command_minion) minion_cert_serial.strip! log "#{target} certificate serial: #{minion_cert_serial}" raise 'Error getting server certificate serial!' unless result_code.zero? raise "Error comparing #{target} certificate with server!" unless minion_cert_serial == server_cert_serial end

Bearing in mind that if you use this suggested loop, you will be iterating over all initialized twopences, so not only some particular targets.

Thanks for the suggestion. I will do some test run with this code.

This broke the code somehow.

Scenario: Check all new server certificates on the minions # features/secondary/srv_rename_hostname.feature:134 This scenario ran at: 2023-10-20 16:53:27 +0200 When I check all certificates after renaming the server hostname # features/step_definitions/command_steps.rb:1550 Server certificate serial: 35:a1:54:c6:5e:c3:f5:f5:c6:d8:52:e6:f9:73:20:a2:d1:c5:75:83 FAIL: openssl x509 --noout --text -in /etc/pki/trust/anchors/RHN-ORG-TRUSTED-SSL-CERT | grep -A1 'Serial' | grep -v 'Serial' returned status code = 1. Output: Can't open /etc/pki/trust/anchors/RHN-ORG-TRUSTED-SSL-CERT for reading, No such file or directory 139797779679040:error:02001002:system library:fopen:No such file or directory:crypto/bio/bss_file.c:69:fopen('/etc/pki/trust/anchors/RHN-ORG-TRUSTED-SSL-CERT','r') 139797779679040:error:2006D080:BIO routines:BIO_new_file:no such file:crypto/bio/bss_file.c:76: unable to load certificate (RuntimeError) ./features/support/lavanda.rb:177:in `run_local' ./features/support/lavanda.rb:155:in `run' ./features/step_definitions/command_steps.rb:1561:in `block (2 levels) in <top (required)>' ./features/step_definitions/command_steps.rb:1560:in `each' ./features/step_definitions/command_steps.rb:1560:in `/^I check all certificates after renaming the server hostname$/' features/secondary/srv_rename_hostname.feature:135:in `I check all certificates after renaming the server hostname' => /var/log/rhn/rhn_web_ui.log

srbarrios

Few suggestions to improve this code, but if it urge to be merged, go ahead.

github-actions · 2023-10-20T13:51:46Z

👋 Hello! Thanks for contributing to our project 😄
☕ Acceptance tests will take same time (aprox. 1h)
👀 Once tests finish, you can check the cucumber report.
See the troubleshooting guide if you need any help.
⚠️ You should not merge if acceptance tests fail to pass
Happy hacking

github-actions · 2023-10-20T15:10:17Z

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take same time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/7700/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/7700/checks.
See the troubleshooting guide if you need any help.

⚠️ You should not merge if acceptance tests fail to pass. ⚠️

Happy hacking!

Signed-off-by: Dominik Gedon <dominik.gedon@suse.com>

github-actions · 2023-10-20T15:21:48Z

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take same time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/7700/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/7700/checks.
See the troubleshooting guide if you need any help.

⚠️ You should not merge if acceptance tests fail to pass. ⚠️

Happy hacking!

nodeg · 2023-10-20T15:22:18Z

Rebase and squashing commits done. The tests already passed (see screenshot above), so I merge without waiting for them again.

nodeg self-assigned this Oct 17, 2023

github-actions bot added secondary-features testing ruby_rubocop test-framework labels Oct 17, 2023

srbarrios reviewed Oct 18, 2023

View reviewed changes

testsuite/features/secondary/srv_rename_hostname.feature Outdated Show resolved Hide resolved

srbarrios reviewed Oct 18, 2023

View reviewed changes

testsuite/features/secondary/srv_rename_hostname.feature Outdated Show resolved Hide resolved

srbarrios reviewed Oct 18, 2023

View reviewed changes

testsuite/features/step_definitions/command_steps.rb Outdated Show resolved Hide resolved

srbarrios reviewed Oct 18, 2023

View reviewed changes

testsuite/features/step_definitions/command_steps.rb Outdated Show resolved Hide resolved

nodeg force-pushed the qe-fix-hostname-renam branch 4 times, most recently from 57ac40a to c1b95a5 Compare October 19, 2023 12:48

nodeg force-pushed the qe-fix-hostname-renam branch 2 times, most recently from 95aa7d9 to 11139ed Compare October 19, 2023 13:00

nodeg marked this pull request as ready for review October 19, 2023 13:54

nodeg requested a review from a team as a code owner October 19, 2023 13:54

nodeg changed the title ~~QE: Fix server hostname rename test~~ QE: Fix and imrpvoe the server hostname rename test Oct 20, 2023

Bischoff approved these changes Oct 20, 2023

View reviewed changes

Bischoff self-requested a review October 20, 2023 09:05

nodeg requested a review from ktsamis October 20, 2023 09:07

Bischoff reviewed Oct 20, 2023

View reviewed changes

nodeg requested a review from srbarrios October 20, 2023 09:38

ktsamis changed the title ~~QE: Fix and imrpvoe the server hostname rename test~~ QE: Fix and improve the server hostname rename test Oct 20, 2023

nodeg force-pushed the qe-fix-hostname-renam branch from 11139ed to 6521d86 Compare October 20, 2023 13:21