Skip to content

Conversation

@ehvs
Copy link
Collaborator

@ehvs ehvs commented Dec 3, 2025

Which issue this PR addresses:

https://issues.redhat.com/browse/ARO-22964
Side-car PR mandatory for #4494

What does it fixes:

Test plan for issue:

  • e2e
  • Compatibility test Tested in a dev cluster calling AdminUpdate.
  1. Created a local dev rp with master branch (without the changes)
  2. Switched to my dev branch and ran the RP
  3. Called AdminUpdate with curl command
    Results:
    a. The ARO_IMAGE for the operator was updated successfully
    b. The MUO image version was updated successfully
    c. The role object was updated.
  • New cluster Built a custom image with the changes and it ran successful.
  • ✅ Unit testing
# go test -v ./pkg/operator/controllers/muo
=== RUN   TestMUOReconciler
=== RUN   TestMUOReconciler/disabled
=== RUN   TestMUOReconciler/managed
=== RUN   TestMUOReconciler/managed,_no_pullspec_(uses_default)
=== RUN   TestMUOReconciler/managed,_MUO_does_not_become_ready
=== RUN   TestMUOReconciler/managed,_could_not_parse_cluster_version_fails
=== RUN   TestMUOReconciler/managed,_CreateOrUpdate()_fails
=== RUN   TestMUOReconciler/managed=false_(removal)
=== RUN   TestMUOReconciler/managed=false_(removal),_Remove()_fails
=== RUN   TestMUOReconciler/managed=blank_(no_action)
--- PASS: TestMUOReconciler (0.00s)
    --- PASS: TestMUOReconciler/disabled (0.00s)
    --- PASS: TestMUOReconciler/managed (0.00s)
    --- PASS: TestMUOReconciler/managed,_no_pullspec_(uses_default) (0.00s)
    --- PASS: TestMUOReconciler/managed,_MUO_does_not_become_ready (0.00s)
    --- PASS: TestMUOReconciler/managed,_could_not_parse_cluster_version_fails (0.00s)
    --- PASS: TestMUOReconciler/managed,_CreateOrUpdate()_fails (0.00s)
    --- PASS: TestMUOReconciler/managed=false_(removal) (0.00s)
    --- PASS: TestMUOReconciler/managed=false_(removal),_Remove()_fails (0.00s)
    --- PASS: TestMUOReconciler/managed=blank_(no_action) (0.00s)
=== RUN   TestDeployCreateOrUpdateCorrectKinds
--- PASS: TestDeployCreateOrUpdateCorrectKinds (0.00s)
=== RUN   TestDeployConfig
--- PASS: TestDeployConfig (0.00s)
PASS
ok      github.com/Azure/ARO-RP/pkg/operator/controllers/muo    0.023s

Other logs

# oc get pods
NAME                                        READY   STATUS    RESTARTS   AGE
managed-upgrade-operator-5cc48fc9f7-c944h   1/1     Running   0          21m


ts=2025-12-11T12:30:42.19783184Z level=info msg="Starting EventSource" controller=upgradeconfig controllerGroup=upgrade.managed.openshift.io controllerKind=UpgradeConfig source="kind source: *v1alpha1.UpgradeConfig"
ts=2025-12-11T12:30:42.19787924Z level=info msg="Starting EventSource" controller=machineconfigpool controllerGroup=machineconfiguration.openshift.io controllerKind=MachineConfigPool source="kind source: *v1.MachineConfigPool"
ts=2025-12-11T12:30:42.19801364Z level=info msg="Starting EventSource" controller=node controllerGroup= controllerKind=Node source="kind source: *v1.Node"
ts=2025-12-11T12:30:42.385416628Z level=info msg="Starting Controller" controller=machineconfigpool controllerGroup=machineconfiguration.openshift.io controllerKind=MachineConfigPool
ts=2025-12-11T12:30:42.385442328Z level=info msg="Starting workers" controller=machineconfigpool controllerGroup=machineconfiguration.openshift.io controllerKind=MachineConfigPool workercount=1
ts=2025-12-11T12:30:42.385449628Z level=info msg="Starting Controller" controller=node controllerGroup= controllerKind=Node
ts=2025-12-11T12:30:42.385460228Z level=info msg="Starting workers" controller=node controllerGroup= controllerKind=Node workercount=1
ts=2025-12-11T12:30:42.385462728Z level=info msg="Starting Controller" controller=upgradeconfig controllerGroup=upgrade.managed.openshift.io controllerKind=UpgradeConfig
ts=2025-12-11T12:30:42.385469028Z level=info msg="Starting workers" controller=upgradeconfig controllerGroup=upgrade.managed.openshift.io controllerKind=UpgradeConfig workercount=1
ts=2025-12-11T12:30:42.385535928Z level=info logger=controller_machineconfigpool msg="Reconciling MachineConfigPool" Request.Namespace= Request.Name=worker

mrWinston
mrWinston previously approved these changes Dec 3, 2025
Copy link
Collaborator

@mrWinston mrWinston left a comment

Choose a reason for hiding this comment

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

Looks good!

tiguelu
tiguelu previously approved these changes Dec 3, 2025
@ehvs ehvs added the hold Hold label Dec 3, 2025
@ehvs ehvs dismissed stale reviews from tiguelu and mrWinston via 4e0ef48 December 11, 2025 12:43
@ehvs
Copy link
Collaborator Author

ehvs commented Dec 11, 2025

The error in e2e is:

run-e2e  |   [TIMEDOUT] ARO Operator - MUO Deployment [It] must be deployed by default with FIPS crypto mandated
run-e2e  |   /app/test/e2e/operator.go:435

This is a test that we are doing inspecting a string g.Expect(body).To(ContainSubstring(X:boringcrypto,strictfipsruntime))
When the logs on the new image does not have strictfipsruntime

ts=2025-12-11T12:19:48.086416145Z level=info logger=cmd msg="Operator Version: 1.0.0"
ts=2025-12-11T12:19:48.086432345Z level=info logger=cmd msg="Go Version: go1.24.9 X:boringcrypto"
ts=2025-12-11T12:19:48.086435745Z level=info logger=cmd msg="Go OS/Arch: linux/amd64"
ts=2025-12-11T12:19:48.086438245Z level=info logger=cmd msg="Version of operator-sdk: v1.21.0"

To fix this will need to fix the validation.
Breadcrumbs
openshift/boilerplate#516
openshift/managed-upgrade-operator#505

@ehvs
Copy link
Collaborator Author

ehvs commented Dec 11, 2025

MIWI failed, rerunning to check if is flake.

run-e2e  |   [FAIL] ARO Operator - dnsmasq [It] must respect the forceReconciliation flag and not update MachineConfigs by default
run-e2e  |   /app/test/e2e/operator.go:695
run-e2e  | 
run-e2e  | Ran 70 of 94 Specs in 5896.345 seconds
run-e2e  | FAIL! -- 69 Passed | 1 Failed | 5 Pending | 19 Skipped

@ehvs ehvs added ready-for-review and removed hold Hold labels Dec 12, 2025
Copy link
Collaborator

@RickARodriguez RickARodriguez left a comment

Choose a reason for hiding this comment

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

This PR was to bump MUO, what does the update to the static resource affect in this instance? Will it be applied on future installs? Or will it get pushed to the operator on an admin update? And are there any concern about backwards comparability?

@ehvs
Copy link
Collaborator Author

ehvs commented Dec 12, 2025

This RP was to bump MUO, what does the update to the static resource affect in this instance? Will it be applied on future installs? Or will it get pushed to the operator on an admin update? And are there any concern about backwards comparability?

Good questions.

  1. the static resource is required because of the change in upstream openshift/muo/#517. (I've updated the PR description)
  2. It will be applied on future installs
  3. Yes, The existing clusters will need an admin-update
  4. My understanding is that there is no concerns for backwards compatibility because once the ARO operator is updated it will reconcile the whole thing and use the new version and settings.

@ehvs
Copy link
Collaborator Author

ehvs commented Jan 5, 2026

Updated the description to include all the testing made.

managedUpgradeOperatorDeployment = "managed-upgrade-operator"
)

It("must be deployed by default with FIPS crypto mandated", func(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So, I get that this test is broken due to the update but is it true that the behavior this test outlined is no longer needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you go into some more detail? Any change related to FIPS makes me extra cautious

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Our check first is a string match on a log message, which is not an ensurance.
The MUO image uses the operator boilerplate, that is actually controlled by the owners of the operator in boilerplate repository.
Currently MUO owners dropped the use of strictfipsruntime because it was giving errors to them, and they are now relying on boringcrypto (check PR description)
This operator is managed by ROSA SRE which is FedRamp compliant, meaning FIPS compliant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants