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

Removal of KubeletConfigFile feature gate: Step 1 #58760

Merged
merged 1 commit into from Jan 26, 2018

Conversation

@mtaufen
Contributor

mtaufen commented Jan 24, 2018

This feature gate was redundant with the --config flag, which already
enables/disables loading Kubelet config from a file.

Since the gate guarded an alpha feature, removing it is not a violation
of our API guidelines.

Some stuff in kubernetes/test-infra currently sets the gate,
so removing will be a 3 step process:

  1. This PR, which makes the gate a no-op.
  2. Stop setting the gate in kubernetes/test-infra. kubernetes/test-infra#6490
  3. Completely remove the gate (this PR will get the release note).
NONE
@@ -105,7 +105,6 @@ type KubeletFlags struct {
// The Kubelet will load its initial configuration from this file.
// The path may be absolute or relative; relative paths are under the Kubelet's current working directory.
// Omit this flag to use the combination of built-in default configuration values and flags.
// To use this flag, the KubeletConfigFile feature gate must be enabled.
KubeletConfigFile flag.StringFlag

This comment has been minimized.

@liggitt

liggitt Jan 24, 2018

Member

can just be a string, and check len(KubeletConfigFile) > 0 instead of Provided()

This comment has been minimized.

@mtaufen

mtaufen Jan 24, 2018

Contributor

sgtm

This comment has been minimized.

@mtaufen

mtaufen Jan 24, 2018

Contributor

note to self: we should do the same for dynamic config dir, but in a separate PR

This comment has been minimized.

@mtaufen

mtaufen Jan 24, 2018

Contributor

done

DynamicKubeletConfig: {Default: false, PreRelease: utilfeature.Alpha},
AppArmor: {Default: true, PreRelease: utilfeature.Beta},
DynamicKubeletConfig: {Default: false, PreRelease: utilfeature.Alpha},
// KubeletConfigFile is now a no-op gate on the path to removal.

This comment has been minimized.

@liggitt

liggitt Jan 24, 2018

Member

minimize diff on the other lines for history's sake, omit comment or put at the end of the line

This comment has been minimized.

@mtaufen

mtaufen Jan 24, 2018

Contributor

done

@liggitt

This comment has been minimized.

Member

liggitt commented Jan 24, 2018

a couple nits, lgtm otherwise

@k8s-ci-robot k8s-ci-robot added size/S and removed size/M labels Jan 24, 2018

Removal of KubeletConfigFile feature gate: Step 1
This feature gate was redundant with the `--config` flag, which already
enables/disables loading Kubelet config from a file.

Since the gate guarded an alpha feature, removing it is not a violation
of our API guidelines.

Some stuff in `kubernetes/test-infra` currently sets the gate,
so removing will be a 3 step process:
1. This PR, which makes the gate a no-op.
2. Stop setting the gate in `kubernetes/test-infra`.
3. Completely remove the gate.

@k8s-ci-robot k8s-ci-robot added size/M and removed size/S labels Jan 24, 2018

@liggitt

This comment has been minimized.

Member

liggitt commented Jan 24, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 24, 2018

@mtaufen

This comment has been minimized.

Contributor

mtaufen commented Jan 24, 2018

/retest

@mtaufen

This comment has been minimized.

Contributor

mtaufen commented Jan 25, 2018

/assign @smarterclayton @vishh
for approval

@dchen1107

This comment has been minimized.

Member

dchen1107 commented Jan 26, 2018

/retest

@dchen1107

This comment has been minimized.

Member

dchen1107 commented Jan 26, 2018

/lgtm

@mtaufen

This comment has been minimized.

Contributor

mtaufen commented Jan 26, 2018

/approve no-issue

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Jan 26, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, liggitt, mtaufen

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Jan 26, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@mtaufen

This comment has been minimized.

Contributor

mtaufen commented Jan 26, 2018

/retest

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Jan 26, 2018

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit 5792214 into kubernetes:master Jan 26, 2018

13 of 14 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-unit
Details
cla/linuxfoundation mtaufen authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-gke-gci Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

mtaufen added a commit to mtaufen/test-infra that referenced this pull request Jan 27, 2018

Stop setting KubeletConfigFile
This feature gate is now a noop, and is no longer necessary to set.
See: kubernetes/kubernetes#58760

mtaufen added a commit to mtaufen/kubernetes that referenced this pull request Jan 29, 2018

Removal of KubeletConfigFile feature gate: Step 3 (final)
This PR completes the work started in
kubernetes#58760
by completely removing the KubeletConfigFile feature gate.

We stopped setting the gate in test-infra in
kubernetes/test-infra#6490.

k8s-merge-robot added a commit that referenced this pull request Jan 29, 2018

Merge pull request #58978 from mtaufen/kc-kubeletconfigfile-step3
Automatic merge from submit-queue (batch tested with PRs 58777, 58978, 58977, 58775). 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>.

Removal of KubeletConfigFile feature gate: Step 3 (final)

This PR completes the work started in
#58760
by completely removing the KubeletConfigFile feature gate.

We stopped setting the gate in test-infra in
kubernetes/test-infra#6490.

```release-note
The alpha KubeletConfigFile feature gate has been removed, because it was redundant with the Kubelet's --config flag. It is no longer necessary to set this gate to use the flag. The --config flag is still considered alpha.
```

iwankgb added a commit to iwankgb/kubernetes that referenced this pull request Feb 15, 2018

Removal of KubeletConfigFile feature gate: Step 3 (final)
This PR completes the work started in
kubernetes#58760
by completely removing the KubeletConfigFile feature gate.

We stopped setting the gate in test-infra in
kubernetes/test-infra#6490.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment