Make ClusterNetwork ClusterNetworkingConfig field optional within #919
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
Cluster resources. This is useful for Managed Control Planes and Bare Metal where networking configuration options may be limited.
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidewatson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/milestone Next |
k8s-ci-robot
added
the
approved
neolit123
reviewed
Apr 29, 2019
| @@ -46,6 +46,7 @@ type Cluster struct { | |||
| // ClusterSpec defines the desired state of Cluster | |||
| type ClusterSpec struct { | |||
| // Cluster network configuration | |||
| // +optional | |||
There was a problem hiding this comment.
it becomes a question whether the API client spec is locked at this point for v1alpha1.
this falls under non-breaking changes, so it seems fine to modify the old config, but it's probably a good idea to clone v1alpha1 into v1alpha2 at this point, setup the roundtrip and start accepting changes there.
a google doc that collects ideas for v1alpha2 changes + a tracking issue that links to it would be nice too.
There was a problem hiding this comment.
Setting up v1alpha2 is a bit more complicated than just roundtripping with apimachinery. It would require either:
- a fully externalized upgrade path, where the v1alpha1 controllers are shut down, the CRDs are migrated to v1alpha2 and then the v1alpha2 controllers are brought up
- a CRD conversion webhook that is built, deployed, and registered. This requires support from controller-runtime: CRD conversion webhooks support controller-runtime#275
k8s-ci-robot
added
the
do-not-merge/hold
vincepri
reviewed
Jun 19, 2019
There was a problem hiding this comment.
/lgtm
/hold
@davidewatson @detiber @neolit123 Any final thoughts on this PR? I'm fine getting this merged on master. We can decide if we want to backport it later.
Feel free to cancel the hold when ready.
k8s-ci-robot
added
the
lgtm
|
+1 to this as well, as some manged-kubernetes service do not request end user to specify network. |
|
+1 |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/test pull-cluster-api-vendor-in-sync |
ncdc
pushed a commit
to ncdc/cluster-api
that referenced
this pull request
Jun 25, 2019
…bernetes-sigs#919) Cluster resources. This is useful for Managed Control Planes and Bare Metal where networking configuration options may be limited. (cherry picked from commit 9e1eb56)
k8s-ci-robot
pushed a commit
that referenced
this pull request
Jun 25, 2019
* Remove duplicate checks for deletionTimestamp (#987) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit aacb0c6) * Rename pkg/controller/controller (#998) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit eafafe8) * Add remote/util.go helpers to work with KubeConfig Secrets (#1004) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit d48025d) * Add remote.ClusterClient to access remote workload clusters (#1006) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 0b25546) * Add events to MachineSet operations (#1012) Add events to the following MachineSet operations - Adopt Machine - Create Machine - Delete Machine (cherry picked from commit b0170a0) * Add patch to events rbac (#1021) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 7365154) * Update some bazel dependencies (#1019) (cherry picked from commit 77836d8) * Update boilerplate check and generated files (#1010) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 0b215f4) * Add CLUSTER_API_KUBECONFIG_READY_TIMEOUT to clusterctl (#1026) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 9cd85f7) * Add events to MachineDeployment operations (#1014) (cherry picked from commit a5e5fdb) * Update controller-runtime and controller-tools (#1032) (cherry picked from commit da61280) * Support for wrapped RequeueAfterError/interface (#1020) This patch adds support for wrapped "RequeueAfterError" errors as well as a new interface for supporting custom "RequeueAfterError" errors. (cherry picked from commit bd0628f) * Use klog in manager and setup controller-runtime logger (#1022) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 0ebf076) * Bazel updates (#1043) - Remove use of deprecated bazel attribute label single_file - Add bazel version checking - Include go_rules fix for cross-compiling darwin binaries (cherry picked from commit 2123eed) * clean up shell scripts in cluster-api (#1045) (cherry picked from commit d63d4be) * Update k8s/code-generator to latest release-1.13 (#1048) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit c2faf86) * downgrade log of util.ExecuteCommand (#975) As this error E0531 is not that ueful and no need to be mark this as 'E', use Info level should be fine as this won't affect return value. (cherry picked from commit 97f25c9) * Make ClusterNetwork ClusterNetworkingConfig field optional within (#919) Cluster resources. This is useful for Managed Control Planes and Bare Metal where networking configuration options may be limited. (cherry picked from commit 9e1eb56) * NodeRef controller (#1011) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 9e89546) * Update generated files, add workaround for controller-tools (#1050) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 1c10a3f) * Update controller-tools to 0.1.11 (#1053) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 1cb3deb) * Update dep check (#1056) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit aecec29) * Fix register bootstrap options in alpha phases (#1064) Fixes issue: #1063 (cherry picked from commit 8141304) * Use remote NodeRef in Machine and MachineSet controllers (#1052) Signed-off-by: Vince Prignano <[email protected]> (cherry picked from commit 1eaf864) * Bazel fixes Signed-off-by: Andy Goldstein <[email protected]>
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.
Cluster resources. This is useful for Managed Control Planes and
Bare Metal where networking configuration options may be limited.
What this PR does / why we need it:
Kubeadm has a default value for
PodCIDRs. This change enables providers which use it to take advantage of the default. Further, in Managed and Bare Metal environments it may not be clear to the user what the podCIDR and serviceCIDR configuration should be.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 #
Special notes for your reviewer:
Note that it is not possible to set defaults using
kubebuilderat this time. Also, I am not sure how we want to manage changes forv1alpha2. Will we make many small changes like this, or do small PRs at this stage clutter up the queue?Release note: