-
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
Add K8s CR name and uuid into NSX resource ID #643
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wenyingd
force-pushed
the
precreated_vpc_naming
branch
3 times, most recently
from
July 19, 2024 03:37
9466b99
to
586c322
Compare
wenyingd
changed the title
Add K8s CR uuid into NSX resource display name
Add K8s CR uuid into NSX resource display name +1
Jul 19, 2024
wenyingd
changed the title
Add K8s CR uuid into NSX resource display name +1
Add K8s CR uuid into NSX resource display name
Jul 19, 2024
wenyingd
requested review from
timdengyun,
zhengxiexie,
heypnus,
lxiaopei and
dantingl
July 19, 2024 11:27
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
July 22, 2024 12:35
d0d4469
to
99acd58
Compare
wenyingd
changed the title
Add K8s CR uuid into NSX resource display name
Add K8s CR uuid into NSX resource display name +1
Jul 23, 2024
wenyingd
changed the title
Add K8s CR uuid into NSX resource display name +1
Add K8s CR uuid into NSX resource display name
Jul 23, 2024
+1 |
wenyingd
force-pushed
the
precreated_vpc_naming
branch
2 times, most recently
from
July 24, 2024 03:12
232aa62
to
9d855d2
Compare
heypnus
reviewed
Jul 24, 2024
timdengyun
reviewed
Jul 24, 2024
@TaoZou1 please take a look at static route part. |
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
July 25, 2024 02:43
9d855d2
to
35ac225
Compare
timdengyun
reviewed
Jul 25, 2024
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
July 25, 2024 03:41
35ac225
to
c6df716
Compare
timdengyun
previously approved these changes
Jul 25, 2024
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 SP part looks good to me.
wenyingd
force-pushed
the
precreated_vpc_naming
branch
2 times, most recently
from
July 29, 2024 07:17
91abaff
to
c28610f
Compare
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
August 5, 2024 05:56
35c7c64
to
0f96045
Compare
TaoZou1
previously approved these changes
Aug 5, 2024
timdengyun
previously approved these changes
Aug 5, 2024
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
August 6, 2024 02:17
0f96045
to
832dc96
Compare
/e2e |
heypnus
previously approved these changes
Aug 6, 2024
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.
subnet/subnetset/subnetport lgtm
/e2e |
1 similar comment
/e2e |
A pre-created VPC is possibly shared in mutiple K8s Namespaces even K8s clusters, which required the corresponding NSX resources created in the same VPC must use unique id. To avoid NSX resources created by the K8s CRs in different Namespaces use the same ID and to improve the readability of the reource ID, this change uses the format ${cr_name}-${cr_uuid} to generate the NSX resource ID. For the scenario that one K8s CR is translated to multiple NSX resources, e.g., Groups, a suffix is added in the ID field to ensure it is unique. In the meanwhile, the prefix which represents the resource type is removed from both the NSX resource id and display_name. Another change is for the VpcSubnet Id generated by SubnetSet CR. The index is changed from a uuid to the hash of a uuid and its length is 8. For the NSX resource's display_name, the corresponding CR's name is used to improve its readability. The exceptions include Subnet and VPC, which uses format ${cr_name}-${cr_uuid}. This is because vCenter create folders for subnet and VPC using its display_name, we use a UUID as a suffix in the display_names field to ensure it is unique. These NSX resources created because of K8s CRs are impacted in this change. - Subnet and Subnetset - SubnetPort - SecurityPolicy and NetworkPolicy - StaticRoute - Group - IPAllocation Test Done: Unit test is added, e2e test on local testbed is passed.
wenyingd
dismissed stale reviews from heypnus, TaoZou1, and timdengyun
via
August 7, 2024 02:55
cc9674c
wenyingd
force-pushed
the
precreated_vpc_naming
branch
from
August 7, 2024 02:55
832dc96
to
cc9674c
Compare
/e2e |
2 similar comments
/e2e |
/e2e |
timdengyun
approved these changes
Aug 7, 2024
dantingl
approved these changes
Aug 7, 2024
heypnus
approved these changes
Aug 7, 2024
timdengyun
added a commit
to timdengyun/nsx-operator-1
that referenced
this pull request
Sep 14, 2024
Previsouly, we use hyphen "-" to connect strings when building NSX resource name, and use underline "_" to connect strings when builindg NSX resource ID. This patch is to unify NSX resource ID and name connecotr as underline when building ID and name from K8s CR. For the NSX resoruce ID and name convention, this patch is follow the standard in PR:vmware-tanzu#643 These NSX resources name are impacted in this change. Subnet SubnetPort SecurityPolicy and NetworkPolicy NSGroup, IPSetGroup and NSRule StaticRoute IPAllocation VPC
timdengyun
added a commit
to timdengyun/nsx-operator-1
that referenced
this pull request
Sep 14, 2024
Previsouly, we use hyphen "-" to connect strings when building NSX resource name, and use underline "_" to connect strings when builindg NSX resource ID. This patch is to unify NSX resource ID and name connecotr as underline when building ID and name from K8s CR. For the NSX resoruce ID and name convention, this patch is follow the standard in PR:vmware-tanzu#643 These NSX resources name are impacted in this change. VPC Subnet SubnetPort SecurityPolicy and NetworkPolicy NSGroup and IPSetGroup NSRule Share StaticRoute IPAllocation
timdengyun
added a commit
to timdengyun/nsx-operator-1
that referenced
this pull request
Sep 14, 2024
Previsouly, we use hyphen "-" to connect strings when building NSX resource name, and use underline "_" to connect strings when builindg NSX resource ID. This patch is to unify NSX resource ID and name connecotr as underline when building ID and name from K8s CR. For the NSX resoruce ID and name convention, this patch is follow the standard in PR:vmware-tanzu#643 These NSX resources name are impacted in this change. VPC Subnet SubnetPort SecurityPolicy and NetworkPolicy NSGroup and IPSetGroup NSRule Share StaticRoute IPAllocation
timdengyun
added a commit
to timdengyun/nsx-operator-1
that referenced
this pull request
Sep 14, 2024
Previsouly, we use hyphen "-" to connect strings when building NSX resource name, and use underline "_" to connect strings when builindg NSX resource ID. This patch is to unify NSX resource ID and name connecotr as underline when building ID and name from K8s CR. For the NSX resoruce ID and name convention, this patch is follow the standard in PR:vmware-tanzu#643 These NSX resources name are impacted in this change: VPC Subnet SubnetPort SecurityPolicy and NetworkPolicy NSGroup and IPSetGroup NSRule Share StaticRoute IPAllocation
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A pre-created VPC is possibly shared in mutiple K8s Namespaces even K8s
clusters, which required the corresponding NSX resources created in the same VPC
must use unique id.
To avoid NSX resources created by the K8s CRs in different Namespaces use the
same ID and to improve the readability of the reource ID, this change uses the
format ${cr_name}-${cr_uuid} to generate the NSX resource ID. For the scenario
that one K8s CR is translated to multiple NSX resources, e.g., Groups, a suffix
is added in the ID field to ensure it is unique. In the meanwhile, the prefix
which represents the resource type is removed from both the NSX resource id and
display_name.
Another change is for the VpcSubnet Id generated by SubnetSet CR. The index is
changed from a uuid to the hash of a uuid and its length is 8.
For the NSX resource's display_name, the corresponding CR's name is used to
improve its readability. The exceptions include Subnet and VPC (NSX LBs is also
changed), which uses format ${cr_name}-${cr_uuid}. This is because vCenter
create folders for subnet and VPC using its display_name, we use a UUID as a
suffix in the display_names field to ensure it is unique.
These NSX resources created because of K8s CRs are impacted in this change.
Test Done:
Unit test is added.
e2e test
- K8s CR: subnet/subnetset
- K8s CR: staticroute
- K8s CR: securitypolicy and networkpolicy (including Groups and Rule)
- K8s CR: subnetport