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

Proposal to support subctl installation via krew #195

Merged
merged 4 commits into from
Jul 4, 2023

Conversation

Jaanki
Copy link
Contributor

@Jaanki Jaanki commented May 8, 2023

Closes: #182

@submariner-bot
Copy link
Collaborator

🤖 Created branch: z_pr195/Jaanki/subctl-krew

@Jaanki Jaanki self-assigned this May 8, 2023
@Jaanki Jaanki requested a review from sridhargaddam May 9, 2023 06:40
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
@Jaanki Jaanki marked this pull request as draft May 10, 2023 07:56
@Jaanki Jaanki requested a review from skitt May 11, 2023 09:50
@Jaanki Jaanki marked this pull request as ready for review May 11, 2023 11:18
@Jaanki Jaanki force-pushed the subctl-krew branch 2 times, most recently from b871ca3 to 4ecda83 Compare May 15, 2023 07:09
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
subctl/issue-182-subctl-a-kubectl-plugin.md Outdated Show resolved Hide resolved
Copy link
Member

@skitt skitt left a comment

Choose a reason for hiding this comment

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

Sorry, I mis-clicked, there are some changes to make still.

@Jaanki Jaanki requested a review from skitt May 26, 2023 10:45
Jaanki added a commit to Jaanki/subctl that referenced this pull request Jun 6, 2023
@Jaanki Jaanki requested a review from tpantelis June 6, 2023 05:34

Once this is done, users would be able to install subctl using krew:

kubectl krew install subm
Copy link
Member

Choose a reason for hiding this comment

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

Can you also include an example on how to install a specific version of subctl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Krew (the package manager for kubectl plugins) does not have built-in support for installing specific versions of a plugin. Krew is designed to fetch and install the latest available version of a plugin from the configured plugin index. However, we can provide multiple versions of their plugins in the plugin index, allowing users to choose which version to install. This facility will not be provided at the moment but can be enhanced in the future.

Copy link
Contributor

@vthapar vthapar left a comment

Choose a reason for hiding this comment

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

Just curious, how would this work with changes proposed in #197 Would we have multiple krew plugins for each build version?


## Summary

krew is a kubectl plugin that makes lifecycle management of kubectl plugins easy. The idea is to have `subctl` as a kubectl plugin and be
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Shouldn't krew be krew?

Copy link
Member

Choose a reason for hiding this comment

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

It should be Krew (see the website).

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be Krew (see the website).

Since one of the goals of this is to use it with oc, would it cause confusion for downstream users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated at other relevant places too.

Copy link
Member

Choose a reason for hiding this comment

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

It should be Krew (see the website).

Since one of the goals of this is to use it with oc, would it cause confusion for downstream users?

Plugins installed using Krew work with both kubectl and oc:

$ oc subm
An installer for Submariner
...

(My kubectl-subm is old.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's also one of the golas: to make subctl (built with build constraints) available downstream via oc as part of OpenShift CLI manager.

and use `subctl` as `kubectl-subm` or `kubectl subm`.

$ kubectl subm
An installer for Submariner
Copy link
Contributor

Choose a reason for hiding this comment

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

Since subctl provides much more than installation, this should be changed to reflect that. Maybe something like:

A utility tool for Submariner

Copy link
Member

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.

nit: I guess Vishal wanted the text to be updated accordingly.

Copy link
Contributor

@vthapar vthapar Jun 22, 2023

Choose a reason for hiding this comment

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

Yes, this text should also be change to make it clear that subctl is more than just an installer.

EDIT: Since text is already changed in subctl output, approving.

@skitt
Copy link
Member

skitt commented Jun 6, 2023

Just curious, how would this work with changes proposed in #197 Would we have multiple krew plugins for each build version?

We’d publish one, full-featured plugin corresponding to upstream subctl.

@Jaanki Jaanki requested a review from vthapar June 7, 2023 13:04
Jaanki added a commit to Jaanki/subctl that referenced this pull request Jun 12, 2023
@Jaanki Jaanki mentioned this pull request Jun 13, 2023
8 tasks
and use `subctl` as `kubectl-subm` or `kubectl subm`.

$ kubectl subm
An installer for Submariner
Copy link
Member

Choose a reason for hiding this comment

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

nit: I guess Vishal wanted the text to be updated accordingly.

and use `subctl` as `kubectl-subm` or `kubectl subm`.

$ kubectl subm
An installer for Submariner
Copy link
Contributor

@vthapar vthapar Jun 22, 2023

Choose a reason for hiding this comment

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

Yes, this text should also be change to make it clear that subctl is more than just an installer.

EDIT: Since text is already changed in subctl output, approving.

@skitt skitt merged commit fdf6173 into submariner-io:devel Jul 4, 2023
@submariner-bot
Copy link
Collaborator

🤖 Closed branches: [z_pr195/Jaanki/subctl-krew]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add subctl as kubectl plugin
6 participants