-
Notifications
You must be signed in to change notification settings - Fork 20
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
update NetworkConfiguration CR #623
base: main
Are you sure you want to change the base?
Conversation
abf2b3b
to
e54d5ac
Compare
@@ -56,11 +56,13 @@ spec: | |||
type: boolean | |||
type: object | |||
accessMode: | |||
default: Private |
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.
Do we set default here or leverage the default value from NetworkConfiguration CR? @lxiaopei
99bdeda
to
eb66a4b
Compare
/e2e |
description: Access mode of Subnet, accessible only from within VPC | ||
or from outside VPC. | ||
enum: | ||
- Private | ||
- Public | ||
- Project |
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.
wcp changed this to PRIVATE_TGW, @lxiaopei shall this type also needs to change
@@ -75,10 +75,10 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |||
return ResultRequeue, err | |||
} | |||
if obj.Spec.AccessMode == "" { | |||
obj.Spec.AccessMode = v1alpha1.AccessMode(vpcNetworkConfig.DefaultSubnetAccessMode) | |||
obj.Spec.AccessMode = v1alpha1.AccessMode(v1alpha1.AccessModePrivate) |
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.
I think we should check if this Accessmode is set for POD or VM
For POD subnetset, we should use vpcNetworkconfiguration.PodSubnetAccessMode
like https://github.com/vmware-tanzu/nsx-operator/pull/627/files#diff-ea8fdc4a9adcc30f1ee646f03f10d565526b3a2b6fec57ae9abaa316c14b8d89R114
we check VM subnetset.
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.
updated.
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.
I mean you need to check this CR is pod-default subnetset or vm-default subnetset. we should not set pod-default subnetset using vpcNetworkconfiguration.PodSubnetAccessMode
6f568f6
to
225e5f5
Compare
if obj.Spec.AccessMode == "" { | ||
obj.Spec.AccessMode = v1alpha1.AccessMode(vpcNetworkConfig.DefaultSubnetAccessMode) | ||
accessMode := v1alpha1.AccessMode(v1alpha1.AccessModePrivate) |
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.
For subnet, we don't need to set PodSubnetAccessMode, since there is no pod subnet created.
subnet is only VM.
For POD, we only have pod-default subnetset. You can confirm with @lxiaopei
759ae3f
to
0fe9fb3
Compare
Signed-off-by: Wenqi Qiu <wenqiq@vmware.com>
0fe9fb3
to
3028d3b
Compare
BTW, could you check why e2e test not pass? |
As this PR uses the new VPC API and the e2e testbed uses the old NSX version, I think it’s expected that the related test cases would fail. |
@lxiaopei pls help confirm some of the comments. |
Create VPC with new VPC API model
Create a Project
wenqi-test
in NSX UICreate a namespace