Bug 1809333: ROKS - remove machine related CRDs from cluster #503
Conversation
|
@csrwng: This pull request references Bugzilla bug 1809333, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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 kubernetes/test-infra repository. |
openshift-ci-robot
added
bugzilla/valid-bug
|
mm @csrwng is unfortunate that this would be overridden every time the CRDs are autogenerated. How are others solving that problem? |
|
@enxebre machine related CRDs are the only ones we need to omit for ROKS. Can we make it part of the CRD generation script? (https://github.com/openshift/machine-api-operator/blob/master/hack/gen-crd.sh) |
openshift-ci-robot
added
size/L
|
@enxebre I updated the gen-crd script to add the annotation for the hosted control plane. If this is acceptable, I'll do the same for the autoscaler PR. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre 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 |
openshift-ci-robot
added
the
approved
|
/retest |
There was a problem hiding this comment.
I'm not a huge fan of the random string encoding differences this Python change introduces, do you know if there's a way to stop it from doing that?
Does any CI run the generation of this code? I ran it locally and had to manually install the yaml dependency, curious as to whether this might break the CI as well?
In a ROKS cluster, the console UI should not show machine related views since machine management is done outside the cluster. The UI will remove the views as long as the CRDs are not present.
openshift-ci-robot
added
size/S
|
Thanks @JoelSpeed. Submitting an alternate implementation that doesn't alter the encoding of the resulting file. It relies on generated CRDs not having any annotations, which I think for now is safe to assume. I don't believe the generate script is used in CI. The checked-in CRD files is what is used everywhere. |
There was a problem hiding this comment.
Looks good, I think you're right, I don't think this is checked by CI, @enxebre do you think we should make sure the generated files are checked in correctly at some point right?
I don't think we can add annotations to CRDs via the CRD generator so I think this should be fine for now. Long term it might be better to replace this with some sort of Kustomize script as that would be more robust, but I think that can wait
/lgtm
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
We don't validate on CI atm, we should though. I just created OCPCLOUD-809 Jira ticket to track this. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@csrwng: All pull requests linked via external trackers have merged. Bugzilla bug 1809333 has been moved to the MODIFIED state. In response to this:
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 kubernetes/test-infra repository. |
|
@csrwng: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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 kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/cherry-pick release-4.4 |
|
@csrwng: new pull request created: #506 In response to this:
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 kubernetes/test-infra repository. |
|
/cherry-pick release4.3 |
|
@csrwng: cannot checkout release4.3: error checking out release4.3: exit status 1. output: error: pathspec 'release4.3' did not match any file(s) known to git. In response to this:
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 kubernetes/test-infra repository. |
|
/cherry-pick release-4.3 |
|
@csrwng: new pull request created: #518 In response to this:
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 kubernetes/test-infra repository. |
In a ROKS cluster, the console UI should not show machine related views since
machine management is done outside the cluster. The UI will remove the
views as long as the CRDs are not present.