Use REX for bats testing in Katello 4.10+ #1695

ianballou · 2023-08-16T18:26:47Z

Switches content actions to use remote execution via ssh instead of Katello agent for Katello 4.10+.

On my local pipeline, ssh keys weren't set up for REX so I added a bit in the setup section to do that.

I haven't yet tested on a pipeline lower than 4.10, but I figured I could get some eyes on this first.

I tested this on my local nightly pipeline from forklift.

evgeni · 2023-08-16T18:30:27Z

We have an installer switch to deploy the keys ;)

forklift/pipelines/vars/forklift_plugins.yml

Line 39 in 48d196a

- "--foreman-proxy-plugin-remote-execution-script-install-key=true"

ianballou · 2023-08-17T19:44:11Z

Katello 4.9 client bats tests passed:

    ok 1 centos8-stream install: enable remote execution (or katello-agent for Katello 4.9 and below)
    ok 2 centos8-stream install: disable puppet agent to prevent checkin from registering host to another org # skip Puppet is not active
    ok 3 centos8-stream install: delete host if present
    ok 4 centos8-stream install: register subscription manager with username and password
    ok 5 centos8-stream install: register subscription manager with activation key
    ok 6 centos8-stream install: start puppet again # skip Puppet isn't enabled
    ok 7 centos8-stream install: check content host is registered
    ok 8 centos8-stream install: enable content view repo
    ok 9 centos8-stream install: install katello-host-tools
    ok 10 centos8-stream install: install package locally
    ok 11 centos8-stream install: check available errata
    ok 12 centos8-stream install: try fetching container content
    ok 13 centos8-stream install: install katello-agent
    ok 14 centos8-stream install: 30 sec of sleep for groggy gofers
    ok 15 centos8-stream install: install package remotely
    ok 16 centos8-stream install: install errata remotely
    ok 17 centos8-stream install: package remove (via remote execution or katello-agent)
    ok 18 centos8-stream install: clean up subscription-manager and gofer after content tests

evgeni · 2023-08-18T06:41:37Z

bats/fb-katello-client.bats

+}
+
+@test "enable remote execution (or katello-agent for Katello 4.9 and below)" {
+ if tIsVersionNewer "${KATELLO_VERSION}" 4.10; then


you could put something like:

tIsKatelloAgentRemoted() { KATELLO_VERSION=$(tKatelloVersion) tIsVersionNewer "${KATELLO_VERSION}" 4.10 }

into the shared files and use that instead of if newer than 4.10 all the time, but up to you :)

evgeni · 2023-08-18T06:43:02Z

bats/fb-katello-client.bats

+ timeout 300 hammer job-invocation create --feature katello_package_install --inputs 'package=gorilla' --search-query "name = ${HOSTNAME}"
+ else
+ timeout 300 hammer host package install --host "${HOSTNAME}" --packages gorilla


I must admit I always found it weird that the REX-based package install is not something like

hammer host package install --host "${HOSTNAME}" --packages gorilla --provider rex

Why does a user need to care about provider?

I think the question is why it needs a job-invocation in one case and a hammer host package install in the other. The user shouldn't have to care: they just want to install a package. It can be nice to have the option to enforce a certain provider for testing though.

I see it as creating a common invocation method no matter the operation. We can have a special invocation method just for package operations, where do we draw the line between special commands and general use of the REX hammer interface?

I am not saying I necessarily like the current REX invocation as it's a bit... wordy. And likely based off the structure of the API more than thinking about the users point of view.

Why does a user need to care about provider?

Exactly.

In the UI, there is a default one and the user can select an alternative one if they want to with a single click.

With hammer, it's totally different depending on the provider.

I definitely agree that it's a bit less convenient than the old katello agent commands. With katello agent gone, we could consider cleaning up the hammer commands to make them more convenient.
@jeremylenz think it's worth an addition to the tasks relating to post katello-agent removal cleanup?

cleaning up the hammer commands to make them more convenient.

What specifically do you have in mind? Keeping the old hammer host package commands and redirecting them to REX? Or improving the hammer job invocation create commands?

See Katello/hammer-cli-katello#904

My approach is to just warn the user and ask them to use the job-invocations
I do not want to be in the business of redirecting because it would require us to keep hammer commands consistent with the job template changes.

ekohl · 2023-08-18T12:28:55Z

bats/fb-katello-client.bats

+ KATELLO_VERSION=$(tKatelloVersion)
+}
+
+@test "enable remote execution (or katello-agent for Katello 4.9 and below)" {


Another approach to make it clear is to have 2 "tests". One for enable katello-agent that's skipped on >= 4.10 and one for enabling REX that's skipped on < 4.10.

Or given how much of this file is conditional: write fb-katello-client-rex.bats and fb-katello-client-katello-agent.bats and handle it at a higher level. Simply don't execute it at all on >= 4.10.

+1 from me to split file approach

bats/fb-katello-client-katello-agent.bats

bats/fb-katello-client-rex.bats

ianballou · 2023-08-21T20:19:53Z

Updated to have two files. Tested on Nightly and Katello 4.9 again.

evgeni · 2023-08-25T06:18:45Z

summarizing for my own sanity, this:

marks the existing "katello-client" tests as being requiring katello agent and skips them on 4.10+
introduces rex-based katello-client tests, having roughly the same feature coverage as the agent based ones and executes them on all versions (good!)
drops the container test (which is not really agent not really rex) from the agent tests

based on that: ACK

side quest:

should the container test be really in an own file, as it is not "client" related (it doesn't require any entitlement or anything) and only really needs the certs to be trusted to work?

ianballou · 2023-08-25T15:33:24Z

summarizing for my own sanity, this:

* marks the existing "katello-client" tests as being requiring katello agent and skips them on 4.10+

* introduces rex-based katello-client tests, having roughly the same feature coverage as the agent based ones and executes them on _all_ versions (good!)

* drops the container test (which is not really agent not really rex) from the agent tests

Sums it up well!

side quest:

* should the container test be really in an own file, as it is not "client" related (it doesn't require any entitlement or anything) and only really needs the certs to be trusted to work?

I can separate it out, sure. We should do container push in the future so those tests can go in there.

ianballou · 2023-08-25T16:35:34Z

@evgeni the container test has been split out.

ianballou force-pushed the remove-katello-agent branch from 8ce7f44 to ecb2b98 Compare August 17, 2023 19:53

evgeni reviewed Aug 18, 2023

View reviewed changes

ekohl reviewed Aug 18, 2023

View reviewed changes

ianballou force-pushed the remove-katello-agent branch from ecb2b98 to 4502398 Compare August 21, 2023 14:36

github-advanced-security bot found potential problems Aug 21, 2023

View reviewed changes

bats/fb-katello-client-katello-agent.bats Fixed Show fixed Hide fixed

bats/fb-katello-client-rex.bats Fixed Show fixed Hide fixed

ianballou force-pushed the remove-katello-agent branch 5 times, most recently from 36bfd0d to d9c3db0 Compare August 21, 2023 19:46

jeremylenz mentioned this pull request Aug 21, 2023

Fixes #36665 - Remove katello-agent API endpoints and rabl nodes Katello/katello#10690

Merged

ianballou requested review from evgeni, ehelms and ekohl August 22, 2023 14:32

ianballou force-pushed the remove-katello-agent branch from d9c3db0 to 682ff8f Compare August 25, 2023 15:40

ianballou force-pushed the remove-katello-agent branch from 682ff8f to acfa739 Compare August 25, 2023 16:36

Use REX for bats testing in Katello 4.10+

acfa739

evgeni approved these changes Aug 25, 2023

View reviewed changes

evgeni merged commit 1ce6bad into theforeman:master Aug 25, 2023
7 of 8 checks passed

ianballou deleted the remove-katello-agent branch August 28, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use REX for bats testing in Katello 4.10+ #1695

Use REX for bats testing in Katello 4.10+ #1695

ianballou commented Aug 16, 2023

evgeni commented Aug 16, 2023

ianballou commented Aug 17, 2023

evgeni Aug 18, 2023

evgeni Aug 18, 2023

ehelms Aug 18, 2023

ekohl Aug 18, 2023

ehelms Aug 18, 2023

evgeni Aug 21, 2023

ianballou Aug 21, 2023

jeremylenz Aug 21, 2023

jeremylenz Aug 21, 2023

parthaa Aug 21, 2023

ekohl Aug 18, 2023

ehelms Aug 18, 2023

ianballou commented Aug 21, 2023

evgeni commented Aug 25, 2023

ianballou commented Aug 25, 2023

ianballou commented Aug 25, 2023

Use REX for bats testing in Katello 4.10+ #1695

Use REX for bats testing in Katello 4.10+ #1695

Conversation

ianballou commented Aug 16, 2023

evgeni commented Aug 16, 2023

ianballou commented Aug 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianballou commented Aug 21, 2023

evgeni commented Aug 25, 2023

ianballou commented Aug 25, 2023

ianballou commented Aug 25, 2023