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

KEP for kubeadm masterconfiguration beta #2131

Merged
merged 1 commit into from
May 15, 2018

Conversation

liztio
Copy link
Contributor

@liztio liztio commented May 4, 2018

No description provided.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 4, 2018
@k8s-ci-robot k8s-ci-robot requested review from jbeda and luxas May 4, 2018 20:26
@k8s-ci-robot
Copy link
Contributor

Hi @liztio. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 4, 2018
@liztio
Copy link
Contributor Author

liztio commented May 4, 2018

/cc @timothysc @kubernetes/sig-cluster-lifecycle-pr-reviews

@k8s-ci-robot
Copy link
Contributor

@liztio: Reiterating the mentions to trigger a notification:
@kubernetes/sig-cluster-lifecycle-pr-reviews

In response to this:

/cc @timothysc @kubernetes/sig-cluster-lifecycle-pr-reviews

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/test-infra repository.

6. Eventually enforce versioning (with failure) on the ConfigMap.

[conversion]: https://godoc.org/k8s.io/apimachinery/pkg/conversion
[v1alpha1]: https://github.com/kubernetes/kubernetes/tree/master/cmd/kubeadm/app/apis/kubeadm
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

We should link the canonical reference to that proper versioned component configs.


Because the configuration is often written and read by different versions of kubeadm compiled by different versions of kubernetes,
it's very important for this configuration file to be well-versioned.

Copy link
Member

@fabriziopandini fabriziopandini May 7, 2018

Choose a reason for hiding this comment

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

I think that there are also some other issues that should be addressed while moving to v1beta1 :

  • stabilisation of the semantic of the API itself
  • better representation of ETCD (in all its variants)
  • introduce a consistent representation of multimaster cluster

@timothysc wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

agreed, I made a similar comment above. I think supporting and properly versioned component config is the 1st step. We can get to beta once we feel that it is ready.


* kubeadm init fails if a configuration file isn't versioned
* the config map written out contains a version
* the configuration struct does not embed any other structs
Copy link
Member

Choose a reason for hiding this comment

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

I'm in favour of extracting other struct from the kubeadm configuration (actually doth kubelet API and kube-proxy API are already stored in separated ConfigMap as well).
However IMO the impact of this move should be better clarified, especially with regards to the impacts on the UX.
e.g. When doing kubeadm init, how the user should pass all the config maps? are all the config maps mandatory?

P.S. A similar discussion is already running on #1915

@timothysc timothysc removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 7, 2018
Right now the configuration format is unversioned.
This means configuration file formats can change between kubeadm versions and there's no safe way to update the configuration format.

We propose a stable version of this configuration, `v1beta1`. Version information will be _mandatory_ going forward, both for user-generated
Copy link
Member

Choose a reason for hiding this comment

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

Can't we just supply v1alpha2 before and ensure proper conversions before we jump to beta. I'd like it to be beta once we have vetted HA configurations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you think this proposal should be for v1alpha2 instead of v1beta1? That sounds fine

## Motivation

After 1.10.0, we discovered a bug in the upgrade process.
The `MasterConfiguraton` embedded a struct that had changed, which caused a backwards-incompatible change to the configuration format.
Copy link
Member

Choose a reason for hiding this comment

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

I'd need to dig more, but in conversing with folks at KubeCon this could be just an in-proper versioning of the kube-proxy component config and if they had properly bumped their version+conversion we may have been ok.

/cc @xiangpengzhao @luxas @thockin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have an alternative at the end of the file. I think we could do this, but we need to step up our testing game if we want to provide guarantees.


Because the configuration is often written and read by different versions of kubeadm compiled by different versions of kubernetes,
it's very important for this configuration file to be well-versioned.

Copy link
Member

Choose a reason for hiding this comment

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

agreed, I made a similar comment above. I think supporting and properly versioned component config is the 1st step. We can get to beta once we feel that it is ready.

6. Eventually enforce versioning (with failure) on the ConfigMap.

[conversion]: https://godoc.org/k8s.io/apimachinery/pkg/conversion
[v1alpha1]: https://github.com/kubernetes/kubernetes/tree/master/cmd/kubeadm/app/apis/kubeadm
Copy link
Member

Choose a reason for hiding this comment

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

We should link the canonical reference to that proper versioned component configs.

* the config map written out contains a version
* the configuration struct does not embed any other structs
* existing configuration files are converted on upgrade to a known, stable version

Copy link
Member

Choose a reason for hiding this comment

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

  • Sparsely populated to leverage omit empty
  • provide sane defaults that can be overridden
    ...

@timothysc
Copy link
Member

@luxas do you have time to review?

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 14, 2018
Automatic merge from submit-queue (batch tested with PRs 63783, 63734). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Rename `kubeadmapiext` to the more explicit `kubeadmapiv1alpha1`

**What this PR does / why we need it**:

`kubeadmext` is somewhat confusing to those who read the code (although it means "the external API of kubeadm", which to some degree makes sense), so I'm swapping all references to it to the more explicit `kubeadmapiv1alpha1`. This change is needed given that we will support multiple external APIs.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: timothysc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2018
@k8s-ci-robot k8s-ci-robot merged commit bd4681b into kubernetes:master May 15, 2018
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 16, 2018
…fig_usage

Automatic merge from submit-queue (batch tested with PRs 63314, 63884, 63799, 63521, 62242). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Restructure internal config usage and fix bugs

**What this PR does / why we need it**:
 - Moves the generic LoadYAML function from the versioned, external API package to a helper library so it can be consumed more easily
 - Makes the upgrading code use the internal version of the API (which always should be used anyway)
 - Moves all config-loading code to `configutil`, together with the migration code needed. This way we have everything in one centralized place, instead of duplicating that logic N times.
 - Makes `kubeadm init` use `configutil` for the reasons mentioned above.

This PR is needed in order to support multiple external API groups (like v1alpha2)

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
This PR depends on:
 - #63782
 - #63783

**Please review only the last (third) commit**

**Release note**:

```release-note
NONE
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 16, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Add (duplicated) v1alpha2 Config API

**What this PR does / why we need it**:
Work in progress PR to add a (initially duplicated) `v1alpha2` we can iterate on during the cycle.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
This PR depends on:
 - [x] #63782
 - [x] #63783
 - [x] #63787
 - [x] #63799

The first commit is from #63799.
The second commit duplicates v1alpha1, but updates timestamps, and doesn't require the `upgrade.go`.
The third commit does the mechanical bump of using v1alpha1 -> v1alpha2
The fourth commit updates bazel

**Release note**:

```release-note
[action required] kubeadm now uses an upgraded API version for the configuration file, `kubeadm.k8s.io/v1alpha2`. kubeadm in v1.11 will still be able to read `v1alpha1` configuration, and will automatically convert the configuration to `v1alpha2` internally and when storing the configuration in the ConfigMap in the cluster.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 16, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

 kubeadm: Remove the `.CloudProvider` and `.PrivilegedPods` configuration option

**What this PR does / why we need it**:
Removes the `.CloudProvider` option, it has been experimental for a long time. People should now use external cloud providers, which is beta in v1.11. Most importantly, you can get the exact same behavior in the API by utilizing the `.*ExtraArgs` and `.*ExtraVolumes` fields.
Removes `.PrivilegedPods` as that serves a super small edge case with the legacy cloud provider, and only for openstack.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
Depends on PRs:
 - [x] #63799
 - [x] #63788

**Release note**:

```release-note
[action required] In the new v1alpha2 kubeadm Configuration API, the `.CloudProvider` and `.PrivilegedPods` fields don't exist anymore.
Instead, you should use the out-of-tree cloud provider implementations which are beta in v1.11.
If you have to use the legacy in-tree cloud providers, you can rearrange your config like the example below.
If you need to use the `.PrivilegedPods` functionality, you can still edit the manifests in
`/etc/kubernetes/manifests/`, and set `.SecurityContext.Privileged=true` for the apiserver
and controller manager.
---
kind: MasterConfiguration
apiVersion: kubeadm.k8s.io/v1alpha2
apiServerExtraArgs:
  cloud-provider: "{cloud}"
  cloud-config: "{path}"
apiServerExtraVolumes:
- name: cloud
  hostPath: "{path}"
  mountPath: "{path}"
controllerManagerExtraArgs:
  cloud-provider: "{cloud}"
  cloud-config: "{path}"
controllerManagerExtraVolumes:
- name: cloud
  hostPath: "{path}"
  mountPath: "{path}"
---
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @dims @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 17, 2018
Automatic merge from submit-queue (batch tested with PRs 63871, 63927, 63966, 63957, 63844). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Remove the never-used .Etcd.SelfHosted field

**What this PR does / why we need it**:
These API types were added to support the self-hosting etcd feature, which in the end never was merged. Hence these API types are unused and should be removed. Perfect timing to do that is now in our new `v1alpha2` scheme.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
Depends on PRs:
 - [x] #63799
 - [x] #63788

**Release note**:

```release-note
kubeadm has removed `.Etcd.SelfHosting` from its configuration API. It was never used in practice.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 20, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add roundtrip, defaulting, upgrading and validation unit tests for the kubeadm API types

**What this PR does / why we need it**:
Follows up from #63799, as well as net-new unit testing for our serialization/deserialization package. This tests our API machinery pretty much end to end.

This is more important now given we now support two external types: #63788

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 21, 2018
Automatic merge from submit-queue (batch tested with PRs 59414, 64096). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Remove `.ImagePullPolicy` from the v1alpha2 API

**What this PR does / why we need it**:
So with `kubeadm config images list/pull` I don't think we need this anymore. Also I don't like this being in our API, as I think the purpose of why it's there can be achieved in other ways.
Instead, I propose to set this explicitely to `IfNotPresent`, and tell the user to prepull the images with `kubeadm config images pull` in case of an airgapped env (or `docker load` ofc) or he/she wants to achieve what `imagePullPolicy: Always` would do. If the images are already cached locally, `IfNotPresent` translates to the same as `Never`, i.e. don't pull (for ppl with no internet connection).


**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of cleaning up the API kubernetes/community#2131

**Special notes for your reviewer**:
This basically reverts: #58960

**Release note**:

```release-note
[action required] kubeadm: The `.ImagePullPolicy` field has been removed in the v1alpha2 API version. Instead it's set statically to `IfNotPresent` for all required images. If you want to always pull the latest images before cluster init (like what `Always` would do), run `kubeadm config images pull` before each `kubeadm init`. If you don't want the kubelet to pull any images at `kubeadm init` time, as you for instance don't have an internet connection, you can also run `kubeadm config images pull` before `kubeadm init` or side-load the images some other way (e.g. `docker load -i image.tar`). Having the images locally cached will result in no pull at runtime, which makes it possible to run without any internet connection.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @rosti @liztio @chuckha
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 22, 2018
Automatic merge from submit-queue (batch tested with PRs 63151, 63795, 63553, 64068, 64113). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Remove .AuthorizationModes in the v1alpha2 API

**What this PR does / why we need it**:
Now that we have #63879, we don't actually need to have `:AuthorizationModes` in our API anymore. This PR removes support for `.AuthorizationModes` in the v1alpha2 API, but keeps an upgrade path available (automatic conversion) from the v1alpha1 version.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
Depends on:
 - [x] #63879
 - [x] #63917

**Release note**:

```release-note
[action required] kubeadm: Support for `.AuthorizationModes` in the kubeadm v1alpha2 API has been removed. Instead, you can use the `.APIServerExtraArgs` and `.APIServerExtraVolumes` fields to achieve the same effect. Files using the v1alpha1 API and setting this field will be automatically upgraded to this v1alpha2 API and the information will be preserved.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 24, 2018
Automatic merge from submit-queue (batch tested with PRs 64127, 63895, 64066, 64215, 64202). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the .Etcd substruct in the v1alpha2 API

**What this PR does / why we need it**:
Splits the monolithic `.Etcd` struct with all the options as fields to a more modular and clear design with two sub-structs for the different modes of hosting etcd we support.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
Depends on:
 - [x] #63917

Follows up: #63871
TODO: I still need to write unit tests for this.

**Release note**:

```release-note
[action required] kubeadm: The `:Etcd` struct has been refactored in the v1alpha2 API. All the options now reside under either `.Etcd.Local` or `.Etcd.External`. Automatic conversions from the v1alpha1 API are supported.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Jun 2, 2018
Automatic merge from submit-queue (batch tested with PRs 64057, 63223, 64346, 64562, 64408). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the Bootstrap Tokens usage in the API types

**What this PR does / why we need it**:
This PR:
 - Moves some common, generic Bootstrap Token helpers and constants from `k8s.io/kubernetes/cmd/kubeadm/app/util/token` to `k8s.io/client-go/tools/bootstrap/token/`
 - Breaks out the top-level Bootstrap Token fields to a dedicated `BootstrapToken` struct with helper functions.
 - Instead of representing the Bootstrap Token as a plain `string`, there is now a wrapper struct `BootstrapTokenString` that can marshal/unmarshal correctly and supports validation on create, and splitting up the full token in the ID/Secret parts automatically.
 - Makes kubeadm support multiple Bootstrap Tokens automatically by supporting a slice of `BootstrapToken` in the `MasterConfiguration` API object
 - Consolidates the place for kubeadm to create token-related flags in an `options` package
 - Supports automatic conversion from the `v1alpha1` to `v1alpha2` API
 - Adds support to set token expiration directly instead of setting a TTL (Expiration and TTL are mutually exclusive)
 - Removes the old `TokenDiscovery` struct we're not using anymore inside of kubeadm

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Related to kubernetes/community#2131

**Special notes for your reviewer**:
This is work in progress. Please only review the first two commits for now.
I will work on splitting up this PR in smaller chunks.
I will also write unit tests tomorrow.

**Release note**:

```release-note
[action required] kubeadm: The Token-related fields in the `MasterConfiguration` object have now been refactored. Instead of the top-level `.Token`, `.TokenTTL`, `.TokenUsages`, `.TokenGroups` fields, there is now a `BootstrapTokens` slice of `BootstrapToken` objects that support the same features under the `.Token`, `.TTL`, `.Usages`, `.Groups` fields.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mattmoyer @liztio
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this pull request Jun 2, 2018
Automatic merge from submit-queue (batch tested with PRs 64057, 63223, 64346, 64562, 64408). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the Bootstrap Tokens usage in the API types

**What this PR does / why we need it**:
This PR:
 - Moves some common, generic Bootstrap Token helpers and constants from `k8s.io/kubernetes/cmd/kubeadm/app/util/token` to `k8s.io/client-go/tools/bootstrap/token/`
 - Breaks out the top-level Bootstrap Token fields to a dedicated `BootstrapToken` struct with helper functions.
 - Instead of representing the Bootstrap Token as a plain `string`, there is now a wrapper struct `BootstrapTokenString` that can marshal/unmarshal correctly and supports validation on create, and splitting up the full token in the ID/Secret parts automatically.
 - Makes kubeadm support multiple Bootstrap Tokens automatically by supporting a slice of `BootstrapToken` in the `MasterConfiguration` API object
 - Consolidates the place for kubeadm to create token-related flags in an `options` package
 - Supports automatic conversion from the `v1alpha1` to `v1alpha2` API
 - Adds support to set token expiration directly instead of setting a TTL (Expiration and TTL are mutually exclusive)
 - Removes the old `TokenDiscovery` struct we're not using anymore inside of kubeadm

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Related to kubernetes/community#2131

**Special notes for your reviewer**:
This is work in progress. Please only review the first two commits for now.
I will work on splitting up this PR in smaller chunks.
I will also write unit tests tomorrow.

**Release note**:

```release-note
[action required] kubeadm: The Token-related fields in the `MasterConfiguration` object have now been refactored. Instead of the top-level `.Token`, `.TokenTTL`, `.TokenUsages`, `.TokenGroups` fields, there is now a `BootstrapTokens` slice of `BootstrapToken` objects that support the same features under the `.Token`, `.TTL`, `.Usages`, `.Groups` fields.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mattmoyer @liztio

Kubernetes-commit: c7b71ebca95d9afb5c4adbadf6cde09a0988d5a7
sttts pushed a commit to sttts/client-go that referenced this pull request Jun 8, 2018
Automatic merge from submit-queue (batch tested with PRs 64057, 63223, 64346, 64562, 64408). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the Bootstrap Tokens usage in the API types

**What this PR does / why we need it**:
This PR:
 - Moves some common, generic Bootstrap Token helpers and constants from `k8s.io/kubernetes/cmd/kubeadm/app/util/token` to `k8s.io/client-go/tools/bootstrap/token/`
 - Breaks out the top-level Bootstrap Token fields to a dedicated `BootstrapToken` struct with helper functions.
 - Instead of representing the Bootstrap Token as a plain `string`, there is now a wrapper struct `BootstrapTokenString` that can marshal/unmarshal correctly and supports validation on create, and splitting up the full token in the ID/Secret parts automatically.
 - Makes kubeadm support multiple Bootstrap Tokens automatically by supporting a slice of `BootstrapToken` in the `MasterConfiguration` API object
 - Consolidates the place for kubeadm to create token-related flags in an `options` package
 - Supports automatic conversion from the `v1alpha1` to `v1alpha2` API
 - Adds support to set token expiration directly instead of setting a TTL (Expiration and TTL are mutually exclusive)
 - Removes the old `TokenDiscovery` struct we're not using anymore inside of kubeadm

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Related to kubernetes/community#2131

**Special notes for your reviewer**:
This is work in progress. Please only review the first two commits for now.
I will work on splitting up this PR in smaller chunks.
I will also write unit tests tomorrow.

**Release note**:

```release-note
[action required] kubeadm: The Token-related fields in the `MasterConfiguration` object have now been refactored. Instead of the top-level `.Token`, `.TokenTTL`, `.TokenUsages`, `.TokenGroups` fields, there is now a `BootstrapTokens` slice of `BootstrapToken` objects that support the same features under the `.Token`, `.TTL`, `.Usages`, `.Groups` fields.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mattmoyer @liztio

Kubernetes-commit: c7b71ebca95d9afb5c4adbadf6cde09a0988d5a7
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this pull request Jun 8, 2018
Automatic merge from submit-queue (batch tested with PRs 64057, 63223, 64346, 64562, 64408). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the Bootstrap Tokens usage in the API types

**What this PR does / why we need it**:
This PR:
 - Moves some common, generic Bootstrap Token helpers and constants from `k8s.io/kubernetes/cmd/kubeadm/app/util/token` to `k8s.io/client-go/tools/bootstrap/token/`
 - Breaks out the top-level Bootstrap Token fields to a dedicated `BootstrapToken` struct with helper functions.
 - Instead of representing the Bootstrap Token as a plain `string`, there is now a wrapper struct `BootstrapTokenString` that can marshal/unmarshal correctly and supports validation on create, and splitting up the full token in the ID/Secret parts automatically.
 - Makes kubeadm support multiple Bootstrap Tokens automatically by supporting a slice of `BootstrapToken` in the `MasterConfiguration` API object
 - Consolidates the place for kubeadm to create token-related flags in an `options` package
 - Supports automatic conversion from the `v1alpha1` to `v1alpha2` API
 - Adds support to set token expiration directly instead of setting a TTL (Expiration and TTL are mutually exclusive)
 - Removes the old `TokenDiscovery` struct we're not using anymore inside of kubeadm

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Related to kubernetes/community#2131

**Special notes for your reviewer**:
This is work in progress. Please only review the first two commits for now.
I will work on splitting up this PR in smaller chunks.
I will also write unit tests tomorrow.

**Release note**:

```release-note
[action required] kubeadm: The Token-related fields in the `MasterConfiguration` object have now been refactored. Instead of the top-level `.Token`, `.TokenTTL`, `.TokenUsages`, `.TokenGroups` fields, there is now a `BootstrapTokens` slice of `BootstrapToken` objects that support the same features under the `.Token`, `.TTL`, `.Usages`, `.Groups` fields.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mattmoyer @liztio

Kubernetes-commit: c7b71ebca95d9afb5c4adbadf6cde09a0988d5a7
calebamiles pushed a commit to calebamiles/community that referenced this pull request Sep 5, 2018
KEP for kubeadm masterconfiguration beta
tamalsaha pushed a commit to kmodules/shared-informer that referenced this pull request Aug 13, 2020
Automatic merge from submit-queue (batch tested with PRs 64057, 63223, 64346, 64562, 64408). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the Bootstrap Tokens usage in the API types

**What this PR does / why we need it**:
This PR:
 - Moves some common, generic Bootstrap Token helpers and constants from `k8s.io/kubernetes/cmd/kubeadm/app/util/token` to `k8s.io/client-go/tools/bootstrap/token/`
 - Breaks out the top-level Bootstrap Token fields to a dedicated `BootstrapToken` struct with helper functions.
 - Instead of representing the Bootstrap Token as a plain `string`, there is now a wrapper struct `BootstrapTokenString` that can marshal/unmarshal correctly and supports validation on create, and splitting up the full token in the ID/Secret parts automatically.
 - Makes kubeadm support multiple Bootstrap Tokens automatically by supporting a slice of `BootstrapToken` in the `MasterConfiguration` API object
 - Consolidates the place for kubeadm to create token-related flags in an `options` package
 - Supports automatic conversion from the `v1alpha1` to `v1alpha2` API
 - Adds support to set token expiration directly instead of setting a TTL (Expiration and TTL are mutually exclusive)
 - Removes the old `TokenDiscovery` struct we're not using anymore inside of kubeadm

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Related to kubernetes/community#2131

**Special notes for your reviewer**:
This is work in progress. Please only review the first two commits for now.
I will work on splitting up this PR in smaller chunks.
I will also write unit tests tomorrow.

**Release note**:

```release-note
[action required] kubeadm: The Token-related fields in the `MasterConfiguration` object have now been refactored. Instead of the top-level `.Token`, `.TokenTTL`, `.TokenUsages`, `.TokenGroups` fields, there is now a `BootstrapTokens` slice of `BootstrapToken` objects that support the same features under the `.Token`, `.TTL`, `.Usages`, `.Groups` fields.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mattmoyer @liztio

Kubernetes-commit: c7b71ebca95d9afb5c4adbadf6cde09a0988d5a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants