-
Notifications
You must be signed in to change notification settings - Fork 294
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
✨ vm-operator: migrate to v1alpha2 #3029
✨ vm-operator: migrate to v1alpha2 #3029
Conversation
39a4c62
to
929ab78
Compare
/test help |
@chrischdi: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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-sigs/prow repository. |
/test pull-cluster-api-provider-vsphere-e2e-govmomi-main |
929ab78
to
173204d
Compare
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main |
173204d
to
3b3f324
Compare
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main |
/assign @sbueringer |
fda1783
to
c5333e2
Compare
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main |
vm.Spec.Network = &vmoprv1.VirtualMachineNetworkSpec{} | ||
} | ||
for _, vnif := range vm.Spec.Network.Interfaces { | ||
if vnif.Network.TypeMeta.GroupVersionKind() == netopv1.SchemeGroupVersion.WithKind("Network") && vnif.Network.Name == network.Name { |
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.
Can we define a const for the network type?
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 const is more or less the api struct, but it cannot be used.
Do we need this somewhere else or why do you think we should consider a struct here?
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.
Note: I've got no strong opinion on it, I am okay to have a const, just want to clarify.
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.
Each network provider has its own network type: Network, VirtualNetwork and SubnetSet. Maybe it is easy to understand the mapping if we have a network type const or have an method GetNetworkType() in the NetworkProvider interface.
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.
added gvk's per type to use instead 👍
c4824b7
to
479c749
Compare
38a4b6f
to
daba150
Compare
Thx! /lgtm /hold for some additional downstream testing |
LGTM label has been added. Git tree hash: 15849023f190eadbcba388fe161e8c19e7eba543
|
/retest |
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main |
daba150
to
1feaf6a
Compare
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main |
@chrischdi: 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-sigs/prow repository. I understand the commands that are listed here. |
/lgtm /hold |
LGTM label has been added. Git tree hash: 836f559f68129a27395db2a2e84ee11023326056
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
Great work! |
/hold cancel |
What this PR does / why we need it:
Updates CAPV to use vm-operators v1alpha2 api version.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Part of #2118