[WIP] Update VM based templates with private IP and update Tiltfile #5248

nawazkh · 2024-11-07T22:58:40Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

This PR updates the templates (based off of VMs) to use private IP of the API Server of the control plane of the workload cluster for inter-node communication.
This PR also updates the Tiltfile with logic to peer AKS VNets with workload cluster's VNets.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes # #5261

Special notes for your reviewer:

cherry-pick candidate

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Update VM based templates to use Private IP solution for workload cluster's inter-node communication. Also update Tiltfile to update AKS Vnet with workload cluster's VNet

k8s-ci-robot · 2024-11-07T22:58:42Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-11-13T00:01:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.53%. Comparing base (7e9ebb9) to head (6cb4ead).
Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5248      +/-   ##
==========================================
- Coverage   53.00%   52.53%   -0.47%     
==========================================
  Files         272      272              
  Lines       29429    29433       +4     
==========================================
- Hits        15598    15463     -135     
- Misses      13027    13167     +140     
+ Partials      804      803       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

mboersma

This seems like a reasonable approach. Should we have somewhere to document which CIDR ranges are "reserved" by which flavors? If we add a new flavor, we'd want to make sure it doesn't overlap, and it would be better not to have that rely just on maintainers' memories.

mboersma · 2024-11-18T18:32:37Z

Tiltfile

    local_resource(
        name = "delete-all-workload-clusters",
-        cmd = kubectl_cmd + " delete clusters --all --wait=false",
+        cmd = ["sh", "-ec", delete_all_workload_clusters],


Do we need to run this command with sh explicitly? It was just kubectl directly before, which seems more portable (although I doubt anyone is running make tilt-up on Windows).

I think we do need sh here. We are updating delete_all_workload_clusters to have a bunch of delete vnet peering commands added in this function https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/5248/files#diff-c2ee8653e1d6b85f0aadf87cd438a9250806c052877248442be4d434cbc52425R522-R532

mboersma · 2024-11-18T18:35:39Z

Tiltfile


    # copy the kubeadm configmap to the calico-system namespace.
    # This is a workaround needed for the calico-node-windows daemonset to be able to run in the calico-system namespace.
    if "windows" in flavor_name:
        flavor_cmd += "; until " + kubectl_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig get configmap kubeadm-config --namespace=kube-system > /dev/null 2>&1; do sleep 5; done"
        flavor_cmd += "; " + kubectl_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig create namespace calico-system --dry-run=client -o yaml | " + kubectl_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig apply -f -; " + kubectl_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig get configmap kubeadm-config --namespace=kube-system -o yaml | sed 's/namespace: kube-system/namespace: calico-system/' | " + kubectl_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig apply -f -"

+    # TODO: no one is clearing MGMT_CLUSTER_NAME when using KIND, so this is always going to be true. Improve this logic.


Could we create a GitHub issue to improve this? It's easy to forget about // TODO comments.

Yes, good call out, will update the epic with it.

Created #5289

k8s-ci-robot · 2024-11-18T19:35:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mboersma. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nawazkh · 2024-11-18T22:33:06Z

Lemme break this PR into multiple smaller PRs updating each of the templates. I will ship this PR with changes to default template and addressing the comments.

nawazkh · 2024-11-18T22:51:40Z

Should we have somewhere to document which CIDR ranges are "reserved" by which flavors? If we add a new flavor, we'd want to make sure it doesn't overlap, and it would be better not to have that rely just on maintainers' memories.

Where could be a good place to put this?
Does a readme in /templates/flavors be a good place to put the CIDR matrix ?

nawazkh · 2024-11-18T23:34:48Z

Something is up with GitHub, it is not letting me see my changes where I dropped my commits to just 5 instead of 23.

k8s-ci-robot · 2024-11-19T00:28:43Z

@nawazkh: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-e2e	`6cb4ead`	link	true	`/test pull-cluster-api-provider-azure-e2e`
pull-cluster-api-provider-azure-windows-with-ci-artifacts	`6cb4ead`	link	false	`/test pull-cluster-api-provider-azure-windows-with-ci-artifacts`
pull-cluster-api-provider-azure-windows-custom-builds	`6cb4ead`	link	false	`/test pull-cluster-api-provider-azure-windows-custom-builds`

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.

nawazkh · 2024-11-19T00:49:26Z

Sorry for the churn @mboersma , I need to close out this PR and open a new one. I will address your comments on there.

nawazkh · 2024-11-19T00:49:28Z

/close

k8s-ci-robot · 2024-11-19T00:49:33Z

@nawazkh: Closed this PR.

In response to this:

/close

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.

nawazkh · 2024-11-19T01:14:57Z

Should we have somewhere to document which CIDR ranges are "reserved" by which flavors? If we add a new flavor, we'd want to make sure it doesn't overlap, and it would be better not to have that rely just on maintainers' memories.

Where could be a good place to put this? Does a readme in /templates/flavors be a good place to put the CIDR matrix ?

Updated https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/5288/files#diff-e7ce6ca4dcc711afa1a2e1afdb50d9c9902749a29f4af50cde1a12f2e51f4858 with the CIDR matrix

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 7, 2024

k8s-ci-robot requested a review from Jont828 November 7, 2024 22:58

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 7, 2024

k8s-ci-robot requested a review from marosset November 7, 2024 22:58

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 7, 2024

nawazkh force-pushed the update_tilt_settings_yaml branch from 7f45e81 to 8718474 Compare November 12, 2024 23:57

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 12, 2024

This was referenced Nov 13, 2024

Make internal LB IP configurable #5264

Open

make internal LB private IP configurable #5269

Closed

nawazkh force-pushed the update_tilt_settings_yaml branch 2 times, most recently from 8eed73c to 40a2e9d Compare November 13, 2024 19:36

nawazkh mentioned this pull request Nov 14, 2024

make Private IP of the internal LB of the API Server configurable #5274

Merged

4 tasks

nawazkh force-pushed the update_tilt_settings_yaml branch from 40a2e9d to 65119d8 Compare November 14, 2024 22:51

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 14, 2024

nawazkh force-pushed the update_tilt_settings_yaml branch from 65119d8 to 0fde5aa Compare November 15, 2024 02:06

nawazkh marked this pull request as ready for review November 15, 2024 03:41

k8s-ci-robot requested a review from jsturtevant November 15, 2024 03:41

nawazkh added 10 commits November 18, 2024 10:45

update ephemeral templates

8a8563d

update private templates

e891464

update dual-stack templates

ef11e75

update ipv6 templates

93ed804

update flatcar templates

d7bf52e

update nvdia-gpu templates

29cedfe

update windows templates

5e29278

update ci/prow-spot templates

ec8f3dc

update ci/prow-custom-vnet templates

040f0f7

fix verify errors

ef039fe

mboersma reviewed Nov 18, 2024

View reviewed changes

assign a /8 cidr to azure cni v1 templates

dfa6e38

nawazkh force-pushed the update_tilt_settings_yaml branch from b5e35e0 to dfa6e38 Compare November 18, 2024 19:35

nawazkh added 2 commits November 18, 2024 11:36

update windows templates with bigger cidr

a31fe3c

allocate bigger dual-stack cidrs

6cb4ead

nawazkh force-pushed the update_tilt_settings_yaml branch from 6cb4ead to d4fcec6 Compare November 18, 2024 22:46

k8s-ci-robot closed this Nov 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Update VM based templates with private IP and update Tiltfile #5248

[WIP] Update VM based templates with private IP and update Tiltfile #5248

nawazkh commented Nov 7, 2024 •

edited

Loading

k8s-ci-robot commented Nov 7, 2024

codecov bot commented Nov 13, 2024 •

edited

Loading

mboersma left a comment

mboersma Nov 18, 2024

nawazkh Nov 18, 2024

mboersma Nov 18, 2024

nawazkh Nov 18, 2024

nawazkh Nov 19, 2024

k8s-ci-robot commented Nov 18, 2024

nawazkh commented Nov 18, 2024

nawazkh commented Nov 18, 2024

nawazkh commented Nov 18, 2024

k8s-ci-robot commented Nov 19, 2024

nawazkh commented Nov 19, 2024

nawazkh commented Nov 19, 2024

k8s-ci-robot commented Nov 19, 2024

nawazkh commented Nov 19, 2024

[WIP] Update VM based templates with private IP and update Tiltfile #5248

[WIP] Update VM based templates with private IP and update Tiltfile #5248

Conversation

nawazkh commented Nov 7, 2024 • edited Loading

k8s-ci-robot commented Nov 7, 2024

codecov bot commented Nov 13, 2024 • edited Loading

Codecov Report

mboersma left a comment

Choose a reason for hiding this comment

mboersma Nov 18, 2024

Choose a reason for hiding this comment

nawazkh Nov 18, 2024

Choose a reason for hiding this comment

mboersma Nov 18, 2024

Choose a reason for hiding this comment

nawazkh Nov 18, 2024

Choose a reason for hiding this comment

nawazkh Nov 19, 2024

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 18, 2024

nawazkh commented Nov 18, 2024

nawazkh commented Nov 18, 2024

nawazkh commented Nov 18, 2024

k8s-ci-robot commented Nov 19, 2024

nawazkh commented Nov 19, 2024

nawazkh commented Nov 19, 2024

k8s-ci-robot commented Nov 19, 2024

nawazkh commented Nov 19, 2024

nawazkh commented Nov 7, 2024 •

edited

Loading

codecov bot commented Nov 13, 2024 •

edited

Loading