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

Deprecate kubelet "enable-controller-attach-detach" option #55517

Open
msau42 opened this issue Nov 11, 2017 · 14 comments
Open

Deprecate kubelet "enable-controller-attach-detach" option #55517

msau42 opened this issue Nov 11, 2017 · 14 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@msau42
Copy link
Member

msau42 commented Nov 11, 2017

Is this a BUG REPORT or FEATURE REQUEST?:
/kind feature
@kubernetes/sig-storage-feature-requests

What happened:
Are there any use cases where we still need to support kubelet doing the attach/detach? If not, let's deprecate this field and eventually remove it.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Nov 11, 2017
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 9, 2018
@msau42
Copy link
Member Author

msau42 commented Feb 27, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 27, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 28, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 27, 2018
@wackxu
Copy link
Contributor

wackxu commented Jun 27, 2018

/remove-lifecycle stale

@wackxu
Copy link
Contributor

wackxu commented Jun 27, 2018

according to #20262,

Since rolling upgrades are only supported for up to two Kubernetes releases, after two releases this rolling upgrade logic can be removed. That means the volume.temporary.kubernetes.io/controller-managed annotation can be retirered and the controller can operate on all nodes by default (ignore the annotation).

I think we can remove the logic for kubelet doing the attach/detach.

@jsafrane
Copy link
Member

It's lengthy process to deprecate something. https://kubernetes.io/docs/reference/using-api/deprecation-policy/

With the old policy (when we introduced the annotation), it would fall into

Rule #7: Deprecated behaviors must function for no less than 1 year after their announced deprecation.

But with the new one, it seems impossible to remove the annotation:

The following rules govern the deprecation of elements of the API. This includes:

  • Annotations on REST resources, including “beta” annotations but not including “alpha” annotations.

Could we get an exception in this case?

@msau42
Copy link
Member Author

msau42 commented Jun 27, 2018

Discussed with @saad-ali a little, he says there are users that still need this flag so I'm not sure if we can ever remove it. If we cannot remove it and need to support it forever, then maybe we need to at least document the limitations of using this, and test this configuration as well.

@Zhuvikin
Copy link

Zhuvikin commented Jul 3, 2018

I think this flag is still needed at least for attach/detach operations in case when some static pod needs volume and controller isn't up yet. My case is to use cinder volume plugin in static pod definition like follows.

volumes:
  - cinder:
      fsType: "ext4"
      volumeID: "1234678-90-1234-5678-90123456789"
    name: data

@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@msau42
Copy link
Member Author

msau42 commented Jul 1, 2019

/reopen
/lifecycle frozen

@k8s-ci-robot
Copy link
Contributor

@msau42: Reopened this issue.

In response to this:

/reopen
/lifecycle frozen

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 reopened this Jul 1, 2019
@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Jul 1, 2019
@msau42
Copy link
Member Author

msau42 commented Jul 1, 2019

This is an untested configuration and as we're adding more features and designing around AD controller/kubelet coordination for things like migration, this functionality is bound to get broken.

We should think about how to support the static pod scenario outlined above considering the move towards CSI and eventually removing all in-tree plugin code. Many CSI features today depend on K8s api objects and won't work in a static pod/master bootstrapping scenario.

@bertinatto
Copy link
Member

@msau42 @jsafrane since we are migrating to CSI and this option is mandatory for that use case, is there a way we can deprecate this now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

No branches or pull requests

7 participants