Skip to content
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

feature: [syncCatalog] Add available Region and Zone Kubernetes Topology Metadata when Syncing Kubernetes Services #3522

Closed

Conversation

kolorful
Copy link
Contributor

@kolorful kolorful commented Jan 26, 2024

Changes proposed in this PR

  • Add available Region and Zone Kubernetes Topology Metadata when Syncing Kubernetes Services

How I've tested this PR

  • Unit test
  • Running locally compiled version on a production Kubernetes cluster and verified it worked (see attached screenshot)
svc1 svc2

How I expect reviewers to test this PR

  • Same as how I tested

Checklist

@kolorful kolorful force-pushed the add-topology-meta-to-node-port-svc branch from 3070b81 to b903fbf Compare January 26, 2024 18:37
@kolorful kolorful changed the title feat: Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services feature: Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services Jan 26, 2024
@kolorful kolorful changed the title feature: Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services feature [syncCatalog]: Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services Jan 26, 2024
@kolorful kolorful changed the title feature [syncCatalog]: Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services feature: [syncCatalog] Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services Jan 26, 2024
@kolorful kolorful force-pushed the add-topology-meta-to-node-port-svc branch 6 times, most recently from 660e568 to f63f74f Compare February 1, 2024 20:24
@david-yu
Copy link
Contributor

david-yu commented Feb 2, 2024

Thanks for the PR. We'll try to see if we can review this after our 1.18 GA is out.

@david-yu
Copy link
Contributor

@kolorful Would this PR instead work for you? It incorporates some of your changes as well: #3693

@kolorful
Copy link
Contributor Author

kolorful commented Feb 28, 2024

@david-yu yes, I'll close mine since #3693 includes my change and added more features to it. Thank @jukie for incorporating the changes to your PR.

@kolorful kolorful closed this Feb 28, 2024
@kolorful kolorful deleted the add-topology-meta-to-node-port-svc branch February 28, 2024 18:31
@david-yu
Copy link
Contributor

david-yu commented Mar 8, 2024

@kolorful could you re-raise this PR? We may want to get this in as #3693 is probably going to take a while to get in since there is a lot more work to do endpoint slice reconciliation, I believe your work gets the topology info from the node as opposed to the endpoint slice which is probably ok.

@kolorful kolorful restored the add-topology-meta-to-node-port-svc branch March 11, 2024 18:25
@kolorful kolorful reopened this Mar 11, 2024
@kolorful
Copy link
Contributor Author

@david-yu, sure I reopend this PR

@kolorful kolorful force-pushed the add-topology-meta-to-node-port-svc branch 3 times, most recently from 3f1780a to 1b54863 Compare March 14, 2024 19:01
@jukie
Copy link
Contributor

jukie commented Mar 14, 2024

Would it be acceptable to add zone/region for all service types?

@kolorful
Copy link
Contributor Author

@jukie sure I added the code and please take a look.

@kolorful kolorful changed the title feature: [syncCatalog] Add Region and Zone Kubernetes Topology Metadata to Synced Node Port Services feature: [syncCatalog] Add available Region and Zone Kubernetes Topology Metadata when Syncing Kubernetes Services Mar 15, 2024
@kolorful kolorful force-pushed the add-topology-meta-to-node-port-svc branch from 7696a00 to a1bd248 Compare March 15, 2024 20:05
@kolorful
Copy link
Contributor Author

This can be closed since #3693 has the change.

@kolorful kolorful closed this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants