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.

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

CMP-2524: Only load node profiles for managed OpenShift #518

Conversation

rhmdnd
Copy link

@rhmdnd rhmdnd commented May 15, 2024

Implement support for only loading node profiles on managed offerings
since the control plane isn't accessible. This reduces the amount of
confusion for users by only presenting profiles that can be run on the
platform.

@openshift-ci-robot
Copy link
Collaborator

@rhmdnd: This pull request references CMP-2524 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

Implement support for only loading node profiles on managed offerings
since the control plane isn't accessible. This reduces the amount of
confusion for users by only presenting profiles that can be run on the
platform.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@rhmdnd rhmdnd requested review from yuumasato and xiaojiey and removed request for mrogers950 May 15, 2024 03:01
@rhmdnd
Copy link
Author

rhmdnd commented May 15, 2024

This still has a rough edge with the kustomization work and detecting platform deployments. Need to find an orthogonal way to handle spinning up tests on platforms.

@rhmdnd
Copy link
Author

rhmdnd commented May 15, 2024

The ROSA test worked... but still need to do some cleanup

2024/05/15 03:34:08 ProfileBundle ready (VALID)
2024/05/15 03:34:13 ProfileBundle ready (VALID)
bypassing MachineConfigPool test setup because it's not supported on rosa
bypassing MachineConfigPool test setup because it's not supported on rosa
=== RUN   TestInstallOnlyParsesNodeProfiles
=== PAUSE TestInstallOnlyParsesNodeProfiles
=== CONT  TestInstallOnlyParsesNodeProfiles
--- PASS: TestInstallOnlyParsesNodeProfiles (0.04s)
PASS
2024/05/15 03:34:18 cleaning up Profile Bundles

Made change to be able to detect what managed cluster we are on using
ClusterClaim and added ROSA platform type.

This makes it easier for users to run the operator on ROSA, without
having to specify the platform. Additional patches can use this to by
smarter about loading in specific resources on ROSA, like profiles and
scan configs.
@rhmdnd rhmdnd force-pushed the CMP-2524-rosa-profile-support branch from 57834fa to ef4c809 Compare May 16, 2024 18:28
Implement support for only loading node profiles on managed offerings
since the control plane isn't accessible. This reduces the amount of
confusion for users by only presenting profiles that can be run on the
platform.
@rhmdnd rhmdnd force-pushed the CMP-2524-rosa-profile-support branch from ef4c809 to 2c7d5f5 Compare May 16, 2024 18:33
@Vincent056
Copy link

/lgtm

Copy link

openshift-ci bot commented May 16, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhmdnd, Vincent056

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit f3ce7cc into ComplianceAsCode:master May 16, 2024
14 checks passed
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request May 22, 2024
We're seeing a panic in the log when parsing profiles on managed
offerings:

  "msg":"odd number of arguments passed as key-value pairs for logging","ignored key"

This is likely related to ComplianceAsCode#518

This commit updates the log to use a key-value pair so that the panic
goes away.
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.

3 participants