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
Bug 1846420: Re-vendor dependencies for manifest and code generation with go modules #72
Conversation
New deep copy code generation Adjust Makefile to use new vendoring New CRDs generated by make manifest target
/test e2e-metal-ipi |
2 similar comments
/test e2e-metal-ipi |
/test e2e-metal-ipi |
@@ -21,7 +21,7 @@ run: generate fmt vet | |||
|
|||
# Install CRDs into a cluster | |||
install: manifests | |||
kubectl apply -f vendor/github.com/openshift/cluster-api/config/crds/ | |||
kubectl apply -f vendor/github.com/openshift/machine-api-operator/install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What CRD(s) in the machine-api-operator does the CAPBM need to function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The definitions of 'machine', 'machinesets', etc come from MAO (and formerly ClusterAPI). Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding /approve because my comment was just a learning question.
/assign @mhrivnak. PTAL at this PR, when you get a chance. A little context. The previous CAPI to MAO PR missed a few things, especially deepcopy and CRD generation. This PR remedies these overlooked parts. Many thanks. |
Is there a gap in CI that explains why it wasn't caught the first time? Has additional testing of any kind been applied to this PR vs the prior one? |
The previous PR passed... our e2e tests run a basic install with control plane and 2 workers, and a subset of the openshift conformance tests. What does this missing affect? |
Also: this needs a BZ so we can backport it to 4.5, the previous PR was against 4.5. |
/approve |
Basically we're not checking a few makefile targets (manifests, CRD and code generation, etc). The e2e worked because the binary built fine and worked correctly. |
Do we have a ticket for adding test jobs for those missing checks? |
We don't. I'll add one right away. |
/retitle Bug 1846420: Re-vendor dependencies for manifest and code generation with go modules |
@kirankt: This pull request references Bugzilla bug 1846420, which is invalid:
Comment 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. |
/bugzilla refresh |
@kirankt: This pull request references Bugzilla bug 1846420, which is invalid:
Comment 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. |
/bugzilla refresh |
@kirankt: This pull request references Bugzilla bug 1846420, which is valid. The bug has been moved to the POST state. 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. |
Can a I get a LGTM, please? I have a PR open to add CI jobs to catch these in the future: This current PR provides the appropriate vendor files for the CI PR to correctly pass its tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/lgtm
/retest Please review the full test history for this PR and help us cut down flakes. |
10 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. |
/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. |
/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. |
/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. |
/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. |
/test e2e-metal-ipi |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 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. |
/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. |
/test e2e-metal-ipi |
/retest Please review the full test history for this PR and help us cut down flakes. |
8 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. |
/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. |
/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. |
/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. |
@kirankt: All pull requests linked via external trackers have merged: openshift/cluster-api-provider-baremetal#72. Bugzilla bug 1846420 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. |
New deep copy code generation
Adjust Makefile to use new vendoring
New CRDs generated by make manifest target