Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Validate workload cluster service CIDR size #2342

Conversation

tylerschultz
Copy link
Contributor

@tylerschultz tylerschultz commented May 10, 2022

What this PR does / why we need it

Kube-apiserver validates the provided service CIDR is not too large.
When provided an invalide service cidr, the kube-apiserver fails
to come up and causes the cluster to hang.

This PR duplicates the service CIDR validation so that cluster creation
with an invalid CIDR fails fast.

The kube-apiserver validation can be seen here:
https://github.com/kubernetes/kubernetes/blob/3c87c43ceff6122637c8d8070601f7271026360e/cmd/kube-apiserver/app/options/validation.go#L52

This validation already exists for management clusters, this cluster CIDR
size validation on workload clusters.

This PR is a follow on to this PR:
#986

Describe testing done for PR

  • Unit tests

  • The cli was built and used to attempt to create a workload cluster with a cluster CIDR that is too large, where it was seen the CLI failed fast.

Release note

Cluster creation validates SERVICE_CIDR size does not exceed `kube-apiserver`'s size restriction, and fails fast. IPv4 CIDRs mask must be `/12` or smaller and IPv6 CIDRs must be `/108` or smaller.

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2342/20220510191505/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@tylerschultz tylerschultz force-pushed the workload-cluster-cidr-size-validation branch from 5cb09df to 36e0950 Compare May 10, 2022 20:54
@tylerschultz tylerschultz changed the title Validate workload cluster CIDR size Validate workload cluster service CIDR size May 10, 2022
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2342/20220510210613/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@tylerschultz tylerschultz force-pushed the workload-cluster-cidr-size-validation branch from 36e0950 to 073a3b0 Compare May 10, 2022 22:49
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2342/20220510230246/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

This validation already exists for managment clusters, this cluster CIDR
size validation on workload clusters.

Kube-apiserver validates the provided service CIDR is not too large.
When provided an invalide service cidr, the kube-apiserver fails
to come up and causes the cluster to hang.

This PR duplicates the service CIDR validation so that cluster creation
with an invalid CIDR fails fast.

The kube-apiserver validation can be seen here:
https://github.com/kubernetes/kubernetes/blob/3c87c43ceff6122637c8d8070601f7271026360e/cmd/kube-apiserver/app/options/validation.go#L52

Signed-off-by: Edwin Xie <exie@vmware.com>
Co-authored-by: Tyler Schultz <tschultz@vmware.com>
@tylerschultz tylerschultz force-pushed the workload-cluster-cidr-size-validation branch from 073a3b0 to 19a52be Compare May 10, 2022 23:34
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2342/20220510234526/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@tylerschultz Since this PR introduces some user-facing changes, can you add a short note under release notes about them?

@tylerschultz
Copy link
Contributor Author

Thanks @vuil. I've added a release note.

@tylerschultz tylerschultz self-assigned this May 12, 2022
@vuil vuil added the ok-to-merge PRs should be labelled with this before merging label May 12, 2022
@tylerschultz tylerschultz merged commit ce80f51 into vmware-tanzu:main May 12, 2022
@tylerschultz tylerschultz deleted the workload-cluster-cidr-size-validation branch May 12, 2022 20:04
ShashankGirish pushed a commit to ShashankGirish/tanzu-framework that referenced this pull request May 25, 2022
This validation already exists for managment clusters, this cluster CIDR
size validation on workload clusters.

Kube-apiserver validates the provided service CIDR is not too large.
When provided an invalide service cidr, the kube-apiserver fails
to come up and causes the cluster to hang.

This PR duplicates the service CIDR validation so that cluster creation
with an invalid CIDR fails fast.

The kube-apiserver validation can be seen here:
https://github.com/kubernetes/kubernetes/blob/3c87c43ceff6122637c8d8070601f7271026360e/cmd/kube-apiserver/app/options/validation.go#L52

Signed-off-by: Edwin Xie <exie@vmware.com>
Co-authored-by: Tyler Schultz <tschultz@vmware.com>

Co-authored-by: Edwin Xie <exie@vmware.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants