Skip to content

Update the Policy CRDs and roles #300

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

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Update the Policy CRDs and roles #300

merged 1 commit into from
Feb 6, 2023

Conversation

mprahl
Copy link
Member

@mprahl mprahl commented Jan 20, 2023

@openshift-ci openshift-ci bot requested review from itdove and yue9944882 January 20, 2023 17:40
@mprahl
Copy link
Member Author

mprahl commented Jan 20, 2023

/cc @dhaiducek

@openshift-ci openshift-ci bot requested a review from dhaiducek January 20, 2023 17:43
@dhaiducek
Copy link
Member

Oh man, I've been neglecting clusteradm updates--we're missing the consolidated sync containers and the PolicyDependency keys here. Do you want to handle all of the reconciles in this PR or would you like to merge this one and I can open a separate PR to handle the rest?

@mprahl mprahl changed the title Update the Policy CRD Update the Policy CRDs Jan 20, 2023
@mprahl
Copy link
Member Author

mprahl commented Jan 20, 2023

@dhaiducek thanks for the reminder. I just synced the Policy CRD as well.

@mikeshng
Copy link
Member

CC @ycyaoxdu

Copy link
Member

@dhaiducek dhaiducek left a comment

Choose a reason for hiding this comment

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

Thanks! Would you also remove the sync images from the addon-controller deployment:

            - name: GOVERNANCE_POLICY_SPEC_SYNC_IMAGE
              value: quay.io/open-cluster-management/governance-policy-spec-sync:{{ .BundleVersion.PolicyAddon }}
            - name: GOVERNANCE_POLICY_STATUS_SYNC_IMAGE
              value: quay.io/open-cluster-management/governance-policy-status-sync:{{ .BundleVersion.PolicyAddon }}
            - name: GOVERNANCE_POLICY_TEMPLATE_SYNC_IMAGE
              value: quay.io/open-cluster-management/governance-policy-template-sync:{{ .BundleVersion.PolicyAddon }}

There were some other minor role updates, but I think they're only relevant to Openshift.

@mprahl
Copy link
Member Author

mprahl commented Jan 20, 2023

name: GOVERNANCE_POLICY_SPEC_SYNC_IMAGE

@dhaiducek I thought we kept those to be able to install older versions of OCM with the latest clusteradm?

@mprahl mprahl changed the title Update the Policy CRDs Update the Policy CRDs and roles Jan 20, 2023
@dhaiducek
Copy link
Member

Oh yeah--we'll definitely need them for backward compatibility. Good call.

@mprahl mprahl requested review from dhaiducek and removed request for yue9944882 and itdove January 20, 2023 21:07
@mprahl
Copy link
Member Author

mprahl commented Jan 20, 2023

@dhaiducek I synced the addon controller and propagator ClusterRoles. Was there anything else that was out of sync?

Copy link
Member

@dhaiducek dhaiducek left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the updates, @mprahl!

@dhaiducek
Copy link
Member

@mprahl The rebase should pick up the OWNERS file that I added, and then we can get the approval for this PR. It looks like your branch doesn't have the commit yet:

@openshift-ci openshift-ci bot added the lgtm label Feb 3, 2023
@dhaiducek
Copy link
Member

Might need a force-push or no-op here. I'm not sure why the e2e-test failed.

@openshift-ci openshift-ci bot removed the lgtm label Feb 3, 2023
@mprahl mprahl requested a review from dhaiducek February 3, 2023 14:05
@mprahl
Copy link
Member Author

mprahl commented Feb 3, 2023

Might need a force-push or no-op here. I'm not sure why the e2e-test failed.

I saw that. I just force pushed again but I noticed another open PR has the same issue.

@openshift-ci openshift-ci bot added the lgtm label Feb 6, 2023
@openshift-ci
Copy link

openshift-ci bot commented Feb 6, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, mprahl

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-robot openshift-merge-robot merged commit 8d00aba into open-cluster-management-io:main Feb 6, 2023
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.

4 participants