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

add init-tiller plugin #87

Closed
wants to merge 1 commit into from
Closed

add init-tiller plugin #87

wants to merge 1 commit into from

Conversation

yogeek
Copy link

@yogeek yogeek commented Mar 4, 2019


Checklist for plugin developers:

  • Read the Plugin Naming Guide (for new plugins)
  • Verify the installation from URL or a local archive works (kubectl krew install --manifest=[...] --archive=[...])

@ahmetb
Copy link
Member

ahmetb commented Mar 4, 2019

I think tiller is going away in new version of Helm. In this case, we probably should not add this plugin.

I recommend you consider proposing this as a plugin to helm (they have a plugin mechanism, too) or as a feature?

@yogeek
Copy link
Author

yogeek commented Mar 4, 2019

@ahmetb thank you for your quick reply. However, as stated in the Helm Roadmap, the version 3 may be added in the indefinite future. In the meantime, numerous teams will still be using V2 version and I think this plugin may be useful.
Maybe we can add it and it will be removed when Helm V3 makes it obsolete ?

@ahmetb
Copy link
Member

ahmetb commented Mar 4, 2019

Maybe we can add it and it will be removed when Helm V3 makes it obsolete?

I believe right now removing a plugin from the index might be breaking krew upgrade and krew delete commands. We haven't tested such a scenario thoroughly.

@yogeek
Copy link
Author

yogeek commented Mar 6, 2019

@ahmetb it would be a shame to refuse plugins because of this abnormal behaviour. Is it possible to test this removal with a fake pugin and to validate this doubt of yours ? If you need help, we can do some tests...

@ahmetb
Copy link
Member

ahmetb commented Mar 6, 2019

I agree with you. Remove seems to work when manifest is gone from the index. However upgrade fails miserably. It's on the roadmap to add a "build receipt" feature that copies the installed manifest.

Details
I0306 11:13:29.015547   45312 scanner.go:74] Reading plugin "warp"
F0306 11:13:29.031695   45312 root.go:50] open /Users/ahmetb/.krew/index/plugins/warp.yaml: no such file or directory
failed to load the index file for plugin
github.com/GoogleContainerTools/krew/cmd/krew/cmd.glob..func4
	/go/src/github.com/GoogleContainerTools/krew/cmd/krew/cmd/upgrade.go:59
github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).execute
	/go/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:762
github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).ExecuteC
	/go/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:852
github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).Execute
	/go/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:800
github.com/GoogleContainerTools/krew/cmd/krew/cmd.Execute
	/go/src/github.com/GoogleContainerTools/krew/cmd/krew/cmd/root.go:48
main.main
	/go/src/github.com/GoogleContainerTools/krew/cmd/krew/main.go:23
runtime.main
	/usr/local/go/src/runtime/proc.go:201
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1333

More importantly, I have a concern that plugins can be one-off install/bootstrap scripts aren't suitable for common availability here. As opposed to a kubectl plugin that users would call frequently, they would call it once per cluster (assuming they aren't going through a lot of clusters).

Historically, I've turned down certain specialized "installation" plugins. Here's an example that (not so surprisingly) installs Helm (in a way very similar to yours). #11 (comment)

image

@yogeek
Copy link
Author

yogeek commented Mar 6, 2019

they would call it once per cluster

More thant that : in fact, I created this plugin because we have to install Tiller once per namespace (to avoid a deployment SPOF), and this for each cluster we have. In this case, it quickly becomes a repetitive tasks... (either for our CI that has to initialize every namespace, or for developers that want to create an ephemeral test namespace to test their helm chart)
But, I get your point and the arguments advanced in the other PRs are defendable...
I will gladly hear from other user's thoughts on this
Anyway, thank you for digging out on the "plugin removal" issue !

@ahmetb
Copy link
Member

ahmetb commented Mar 6, 2019

Apologies for this situation. Right now me being the only maintainer of the index repo isn't helping but I have an outstanding proposal to transfer the project to Kubernetes SIG CLI (https://groups.google.com/forum/#!topic/kubernetes-sig-cli/9fWm_aZ-AC8).

I don't want to accept plugins that might be ineligible later if/when the community comes up with criteria or acceptance rubric that prohibits such bootstrap/installation plugins. And accepting this plugin would set a bad precedent.

Your plugin submission is still a great input to the criteria process the community would come up with later on. So thanks for that. I think you may still be able to distribute your plugin with krew's --manifest, --archive flags but you would not get any upgrades.

@yogeek
Copy link
Author

yogeek commented Mar 7, 2019

@ahmetb No worry, I understand ;) Let's put this in standby for now until the community decides the direction of the project.
now, it is quite disturbing for a newcomer to both solutions coexisting : the kubectl plugin feature on on hand (documented officially), and krew on the other hand, that it much more easier to use but not offically supported. So let's hope krew will become soon a part of k8s native ecosystem !
Thank you for your help :-)

@ahmetb
Copy link
Member

ahmetb commented Apr 8, 2019

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2019
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 8, 2019
@bacongobbler
Copy link

Even if Helm 3 is released today, there will still be a large number of users running Helm 2 in production, and Helm 2 would continue to be supported for a given amount of time until we decide to drop support. I don't see a reason why this shouldn't be available for those users still running Helm 2.

@ahmetb
Copy link
Member

ahmetb commented May 16, 2019

@bacongobbler Reiterating the discussion above, the reason I'm objecting to this isn't because Tiller is going away vs not; it's because I don't think we should have one-off plugins.

Plugins are meant to encapsulate common day-to-day workflows, and installation/bootstrapping operations are more suitable for separate tasks for a script.

@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 Aug 14, 2019
@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

@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 Sep 13, 2019
@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

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closed this PR.

In response to this:

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

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.

@huazhihao huazhihao mentioned this pull request Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/gray-area lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants