Repository: osbuild/osbuild-composer
Pull request #3432 (2023-05-10 15:12:15)

internal/manifest: install rhc-worker-playbook when using rhc
This isn't pulled in as a dependency, and is required for services lik remediations.



croissanne: > Ack. But it seems like it should be pulled in by rhc or insights eventually, shouldn't it?

Indeed
ondrejbudai: > @bcl "Eventually" is the key thing.
> 
> `rhc connect` registers with subscription-manager, registers with insights-client, and connects to MQTT — that's it. `rhc-worker-playbook` might eventually subsequently be installed. But that depends on a lot of other things all working nicely together: the config-manager application, kafka queues, MQTT queues, etc. And even when it's all working nicely, it still takes a minimum of ~60 seconds.

I think @bcl had something different in mind. Shouldn't `rhc` simply require `rhc-worker-playbook` as an RPM dependency? Are there any downsides to this approach?
Ichimonji10: RHC is useful without rhc-worker-playbook:

1. RHC ships out of the box on RHEL, with the intent of being used as the default registration tool. The goal is for a user to be able to execute `rhc connect` instead of `subscription-manager register && insights-client --register`.
2. RHC and foreman_rh_cloud (?) are used by Satellite 6.11+ (and optionally 6.10, IIRC) to execute playbooks.

For these two use cases respectively:

1. Giving `rhc` a hard dependency on `rhc-worker-playbook` means that _all_ RHEL images, even minimal ones, will include an additional ~17 packages. This is a form of bloat that will make it incrementally harder for us to validate, ship, and support RHEL releases.
2. I expect Satellite system administrators would be very upset to see a tool trying to run [Ansible playbooks](https://github.com/RedHatInsights/config-manager/tree/master/playbooks) on their servers.
croissanne: Honestly maybe we shouldn't install this by default either then, and just let rhc do it's thing and let it fetch the playbooks itself. But something to be resolved after, there's reasons rn why this has to land sooner rather than later ._.
Ichimonji10: My general recommendation is that anyone wishing to execute playbooks with RHC explicitly install rhc-worker-playbook. I'm not familiar with osbuild-composer or its requirements, and am happy to hop on a call to see if that general advice fits this specific application and its requirements.

Patch:
diff --git a/internal/manifest/os.go b/internal/manifest/os.go
index 66c06207c..c1e9ffbb5 100644
--- a/internal/manifest/os.go
+++ b/internal/manifest/os.go
@@ -212,7 +212,7 @@ func (p *OS) getPackageSetChain() []rpmmd.PackageSet {
 	if p.Subscription != nil {
 		packages = append(packages, "subscription-manager")
 		if p.Subscription.Rhc {
-			packages = append(packages, "rhc", "insights-client")
+			packages = append(packages, "rhc", "insights-client", "rhc-worker-playbook")
 		} else if p.Subscription.Insights {
 			packages = append(packages, "insights-client")
 		}
@@ -499,12 +499,16 @@ func (p *OS) serialize() osbuild.Pipeline {
 		if p.Subscription.Rhc {
 			// Use rhc for registration instead of subscription manager
 			commands = []string{fmt.Sprintf("/usr/bin/rhc connect -o=%s -a=%s --server %s", p.Subscription.Organization, p.Subscription.ActivationKey, p.Subscription.ServerUrl)}
+			// insights-client creates the .gnupg directory during boot process, and is labeled incorrectly
+			commands = append(commands, "restorecon -R /root/.gnupg")
 		} else {
 			commands = []string{fmt.Sprintf("/usr/sbin/subscription-manager register --org=%s --activationkey=%s --serverurl %s --baseurl %s", p.Subscription.Organization, p.Subscription.ActivationKey, p.Subscription.ServerUrl, p.Subscription.BaseUrl)}
 
 			// Insights is optional when using subscription-manager
 			if p.Subscription.Insights {
 				commands = append(commands, "/usr/bin/insights-client --register")
+				// insights-client creates the .gnupg directory during boot process, and is labeled incorrectly
+				commands = append(commands, "restorecon -R /root/.gnupg")
 			}
 		}
 


Test patch:
diff --git a/internal/manifest/os_test.go b/internal/manifest/os_test.go
index f2843c795..08ab24853 100644
--- a/internal/manifest/os_test.go
+++ b/internal/manifest/os_test.go
@@ -94,6 +94,7 @@ func TestSubscriptionManagerInsightsCommands(t *testing.T) {
 	CheckFirstBootStageOptions(t, pipeline.Stages, []string{
 		"/usr/sbin/subscription-manager register --org=2040324 --activationkey=my-secret-key --serverurl subscription.rhsm.redhat.com --baseurl http://cdn.redhat.com/",
 		"/usr/bin/insights-client --register",
+		"restorecon -R /root/.gnupg",
 	})
 }
 
@@ -110,6 +111,7 @@ func TestRhcInsightsCommands(t *testing.T) {
 	pipeline := os.serialize()
 	CheckFirstBootStageOptions(t, pipeline.Stages, []string{
 		"/usr/bin/rhc connect -o=2040324 -a=my-secret-key --server subscription.rhsm.redhat.com",
+		"restorecon -R /root/.gnupg",
 	})
 }
 
diff --git a/test/cases/api.sh b/test/cases/api.sh
index 6419a0b53..dc44ad027 100755
--- a/test/cases/api.sh
+++ b/test/cases/api.sh
@@ -291,7 +291,7 @@ TEST_ID="$(uuidgen)"
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
   # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 fi
 export TEST_ID
 
diff --git a/test/cases/aws.sh b/test/cases/aws.sh
index c9c30265e..6277ce02c 100755
--- a/test/cases/aws.sh
+++ b/test/cases/aws.sh
@@ -42,7 +42,7 @@ trap cleanup EXIT
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
   # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 else
   # if not running in Jenkins, generate ID not relying on specific env variables
   TEST_ID=$(uuidgen);
diff --git a/test/cases/aws_s3.sh b/test/cases/aws_s3.sh
index 9ed343526..5e1634b28 100755
--- a/test/cases/aws_s3.sh
+++ b/test/cases/aws_s3.sh
@@ -33,7 +33,7 @@ trap cleanup EXIT
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
   # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 else
   # if not running in Jenkins, generate ID not relying on specific env variables
   TEST_ID=$(uuidgen);
diff --git a/test/cases/azure.sh b/test/cases/azure.sh
index 80f2b340b..abba754e2 100755
--- a/test/cases/azure.sh
+++ b/test/cases/azure.sh
@@ -14,7 +14,7 @@ source /usr/libexec/osbuild-composer-test/set-env-variables.sh
 source /usr/libexec/tests/osbuild-composer/shared_lib.sh
 
 BRANCH_NAME="${CI_COMMIT_BRANCH:-local}"
-BUILD_ID="${CI_BUILD_ID:-$(uuidgen)}"
+BUILD_ID="${CI_JOB_ID:-$(uuidgen)}"
 HYPER_V_GEN="${HYPER_V_GEN:-V1}"
 
 # Container image used for cloud provider CLI tools
diff --git a/test/cases/container-embedding.sh b/test/cases/container-embedding.sh
index 35b5ab805..de71bc124 100755
--- a/test/cases/container-embedding.sh
+++ b/test/cases/container-embedding.sh
@@ -45,7 +45,7 @@ get_compose_metadata () {
 }
 
 BRANCH_NAME="${CI_COMMIT_BRANCH:-local}"
-BUILD_ID="${CI_BUILD_ID:-$(uuidgen)}"
+BUILD_ID="${CI_JOB_ID:-$(uuidgen)}"
 TEST_ID="$DISTRO_CODE-$ARCH-$BRANCH_NAME-$BUILD_ID"
 IMAGE_KEY=container-${TEST_ID}
 
diff --git a/test/cases/container-upload.sh b/test/cases/container-upload.sh
index ee75a0815..f0aceed7c 100755
--- a/test/cases/container-upload.sh
+++ b/test/cases/container-upload.sh
@@ -51,7 +51,7 @@ if [ "$ID" != "fedora" ]; then
 fi
 
 BRANCH_NAME="${CI_COMMIT_BRANCH:-local}"
-BUILD_ID="${CI_BUILD_ID:-$(uuidgen)}"
+BUILD_ID="${CI_JOB_ID:-$(uuidgen)}"
 TEST_ID="$DISTRO_CODE-$ARCH-$BRANCH_NAME-$BUILD_ID"
 IMAGE_KEY=container-${TEST_ID}
 
diff --git a/test/cases/gcp.sh b/test/cases/gcp.sh
index ae37c69b1..79b48ed25 100755
--- a/test/cases/gcp.sh
+++ b/test/cases/gcp.sh
@@ -56,7 +56,7 @@ trap cleanup EXIT
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
     # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-    TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+    TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 else
     # if not running in Jenkins, generate ID not relying on specific env variables
     TEST_ID=$(uuidgen);
diff --git a/test/cases/generic_s3.sh b/test/cases/generic_s3.sh
index 572e7f60f..10f53a5e4 100755
--- a/test/cases/generic_s3.sh
+++ b/test/cases/generic_s3.sh
@@ -34,7 +34,7 @@ trap cleanup EXIT
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
     # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-    TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+    TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 else
     # if not running in Jenkins, generate ID not relying on specific env variables
     TEST_ID=$(uuidgen);
diff --git a/test/cases/image_tests.sh b/test/cases/image_tests.sh
index a81a737cd..911a4e859 100755
--- a/test/cases/image_tests.sh
+++ b/test/cases/image_tests.sh
@@ -5,8 +5,8 @@ set -euo pipefail
 source /usr/libexec/osbuild-composer-test/set-env-variables.sh
 source /usr/libexec/tests/osbuild-composer/shared_lib.sh
 
-if [[ -n "$CI_BUILD_ID" ]]; then
-    BUILD_ID="${BUILD_ID:-${CI_BUILD_ID}}"
+if [[ -n "$CI_JOB_ID" ]]; then
+    BUILD_ID="${BUILD_ID:-${CI_JOB_ID}}"
 else
     BUILD_ID="${BUILD_ID:-$(uuidgen)}"
 fi
diff --git a/test/cases/ostree-simplified-installer.sh b/test/cases/ostree-simplified-installer.sh
index 0558a7be4..6730552c1 100755
--- a/test/cases/ostree-simplified-installer.sh
+++ b/test/cases/ostree-simplified-installer.sh
@@ -17,7 +17,7 @@ sudo pip3 install yq==v3.2.1
 sudo dnf install -y fdo-admin-cli
 # Start fdo-aio to have /etc/fdo/aio folder
 sudo systemctl enable --now fdo-aio
-# Prepare service api server config file
+# Prepare service api server config filef
 sudo /usr/local/bin/yq -iy '.service_info.diskencryption_clevis |= [{disk_label: "/dev/vda4", reencrypt: true, binding: {pin: "tpm2", config: "{}"}}]' /etc/fdo/aio/configs/serviceinfo_api_server.yml
 sudo systemctl restart fdo-aio
 
@@ -668,7 +668,7 @@ done
 if [[ "${ANSIBLE_USER}" == "fdouser" ]]; then
     greenprint "🕹 Check if user 'fdouser' exist in edge vm"
     for _ in $(seq 0 30); do
-        FDOUSER_EXIST=$(ssh "${SSH_OPTIONS[@]}" -i "${SSH_KEY}" admin@$PUB_KEY_GUEST_ADDRESS "grep fdouser /etc/passwd")
+        FDOUSER_EXIST=$(ssh "${SSH_OPTIONS[@]}" -i "${SSH_KEY}" admin@$PUB_KEY_GUEST_ADDRESS "grep fdouser /etc/passwd || true")
         if [[ ${FDOUSER_EXIST} =~ "fdouser" ]]; then
             echo "fdouser has been created"
             break
diff --git a/tools/generic_s3_test.sh b/tools/generic_s3_test.sh
index a4fa3d9f8..05b7fc52f 100755
--- a/tools/generic_s3_test.sh
+++ b/tools/generic_s3_test.sh
@@ -52,7 +52,7 @@ trap cleanup EXIT
 CI="${CI:-false}"
 if [[ "$CI" == true ]]; then
   # in CI, imitate GenerateCIArtifactName() from internal/test/helpers.go
-  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_BUILD_ID"
+  TEST_ID="$DISTRO_CODE-$ARCH-$CI_COMMIT_BRANCH-$CI_JOB_ID"
 else
   # if not running in Jenkins, generate ID not relying on specific env variables
   TEST_ID=$(uuidgen);
diff --git a/tools/libvirt_test.sh b/tools/libvirt_test.sh
index 7a3a640c2..22cbc83c3 100755
--- a/tools/libvirt_test.sh
+++ b/tools/libvirt_test.sh
@@ -283,14 +283,14 @@ else
     esac
 
     case "${ID}-${VERSION_ID}" in
-        "rhel-8"* | "rhel-9.0" | "rhel-9.1" | "centos-8")
+        "rhel-8"* | "centos-8")
              NVRAM_TEMPLATE="nvram_template=/usr/share/edk2/ovmf/OVMF_VARS.fd"
              ;;
         "centos-9" )
             # Disable secure boot for CS9 due to bug bz#2108646
             NVRAM_TEMPLATE="firmware.feature0.name=secure-boot,firmware.feature0.enabled=no"
             ;;
-        "rhel-9.2")
+        "rhel-9"*)
              NVRAM_TEMPLATE=""
              ;;
         *)