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 kubectl kubernetes client wrapper #1302

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

afbjorklund
Copy link
Member

Will connect to a running "k3s" or "k8s" instance.

Similar to other wrappers, but uses conf not sock.

Issue #1300

Needs PR #1301 in order to create the "conf" files

cmd/kubectl.lima Outdated Show resolved Hide resolved
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM, but maybe the error message can be a bit more helpful?

@afbjorklund
Copy link
Member Author

It at least outputs the name now, but it doesn't mention which template to use...

$ ./cmd/docker.lima 
instance "docker" does not exist, run `limactl start --name=docker template://docker` to create a new instance
$ ./cmd/podman.lima 
instance "podman" does not exist, run `limactl start --name=podman template://podman` to create a new instance
$ ./cmd/kubectl.lima 
no running instance found, run `limactl start` to start an instance
looked for instance names "k3s" and "k8s", maybe set LIMA_INSTANCE?

Maybe should add similar output here, and check if it exists or if it is just stopped...

@jandubois
Copy link
Member

Maybe should add similar output here, and check if it exists or if it is just stopped...

Yeah, probably should.

No k3s or k8s running instances found. Either start one with
limactl start --name=k3s template://k3s
limactl start --name=k8s template://k8s
or set LIMA_INSTANCE to the name of your Kubernetes instance.

It quickly gets complicated when you want to check for stopped instances, and then recommand to start those instead of creating a new one, but maybe try to see if you can write it concisely?

@jandubois
Copy link
Member

Note that the other scripts don't catch it if an instance exists, but hasn't been started. But as you wrote earlier, I think the error message you get in that case should be sufficient already.

@afbjorklund afbjorklund marked this pull request as ready for review January 17, 2023 16:25
@jandubois
Copy link
Member

I haven't tested again right now, but LGTM. But it requires #1301 to be final before we can approve/merge this, as the name of the conf directory may still change.

Will connect to a running "k3s" or "k8s" instance.

Similar to other wrappers, but uses conf not sock.

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Requires #1301 to be merged

@jandubois jandubois merged commit 49bfb19 into lima-vm:master Jan 31, 2023
@AkihiroSuda AkihiroSuda added this to the v0.15 (tentative) milestone Jan 31, 2023
@AkihiroSuda
Copy link
Member

Next time please set the milestone before merging

@jandubois
Copy link
Member

Next time please set the milestone before merging

Sorry about that, I normally do, but I guess I missed it because most of the current PRs already had a milestone assigned, so I forgot to double-check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants