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

[Housekeeping] Add precheck in demo and sandox commands for local kubeconfig #2943

Closed
2 tasks done
Tracked by #2917
pmahindrakar-oss opened this issue Oct 3, 2022 · 3 comments
Closed
2 tasks done
Tracked by #2917
Labels
flytectl Issues related to flytectl -Flytes CLI hacktoberfest housekeeping Issues that help maintain flyte and keep it tech-debt free
Milestone

Comments

@pmahindrakar-oss
Copy link
Contributor

Describe the issue

sandbox and demo clusters copies the kubecontext to connect to the cluster into the local machines default kubecontext and switches over to it.

In case the kubecontext doesn't exist locally then the command fails at the this step which is after sandbox is successfully up

Add this precheck for demo and sandbox both
https://github.com/flyteorg/flytectl/blob/master/cmd/demo/start.go#L79
https://github.com/flyteorg/flytectl/blob/master/cmd/sandbox/start.go#L83

What if we do not do this?

User experience is bad as it will always fail for him

Related component(s)

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@pmahindrakar-oss pmahindrakar-oss added the housekeeping Issues that help maintain flyte and keep it tech-debt free label Oct 3, 2022
@pmahindrakar-oss pmahindrakar-oss added this to the 1.3.0 milestone Oct 3, 2022
@pmahindrakar-oss pmahindrakar-oss added the flytectl Issues related to flytectl -Flytes CLI label Oct 3, 2022
@Icelain
Copy link

Icelain commented Oct 3, 2022

By this do you mean to check if the Config struct here is empty/non optional fields are filled? - https://github.com/flyteorg/flytectl/blob/master/cmd/config/subcommand/sandbox/sandbox_config.go#L6

@pmahindrakar-oss
Copy link
Contributor Author

@Icelain I meant we don't do a check for local kubecontext and use the k8sclient to copy over the sandbox context to local kube context https://github.com/flyteorg/flytectl/blob/d2baa0907f7b4ca527bbcc5677483ae8692fd7e6/pkg/sandbox/start.go#L265
and which is after all the sandbox cluster is up and running and fails at these last few steps.

Instead of failing here, it should fail early by validating the existence of the local kube context and if none exist then fail the sandbox/demo cluster creation as these are prerequisite for this command

The task here is to add this validation

@daniel-shuy
Copy link
Contributor

I've created a PR for this at flyteorg/flytectl#363

@pmahindrakar-oss can you help to review it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flytectl Issues related to flytectl -Flytes CLI hacktoberfest housekeeping Issues that help maintain flyte and keep it tech-debt free
Projects
None yet
Development

No branches or pull requests

4 participants