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

k0s reset: don't use crictl as a dependency #772

Merged

Conversation

jewertow
Copy link
Contributor

@jewertow jewertow commented Mar 11, 2021

Signed-off-by: Jacek Ewertowski jacek.ewertowski1@gmail.com

Issue
Fixes #743

What this PR Includes

  1. Custom implementation of crictl commands.
  2. Removal of embedded crictl.

How to test this change

  1. Run k0s:
k0s install controller --enable-worker
systemctl start k0scontroller
  1. Wait for k0s pods to be ready. There should be 6 pods:
systemctl status k0scontroller
k0s kubectl get pods -n kube-system
  1. Stop k0s and reset:
systemctl stop k0scontroller
k0s reset --debug

All containers should be stopped and removed successfully.

@jewertow jewertow requested a review from a team as a code owner March 11, 2021 23:30
@jewertow jewertow changed the title k0s reset: don't use crictl as a dependency [WIP] k0s reset: don't use crictl as a dependency Mar 11, 2021
@jewertow jewertow force-pushed the 743-dont-use-crictl-as-a-dependency branch from 1c56eeb to a9fde7b Compare March 21, 2021 16:51
@jewertow jewertow changed the title [WIP] k0s reset: don't use crictl as a dependency k0s reset: don't use crictl as a dependency Mar 21, 2021
@jasmingacic
Copy link
Contributor

@jewertow can you fix the build so we can proceed with the review

@jewertow
Copy link
Contributor Author

@jasmingacic it seems to be a random failure. Are you able to rerun failed CI plan or do I have to trigger CI by empty commits?

@jasmingacic
Copy link
Contributor

@jewertow I re-ran it and it failed again. No need to push any more commits.

Can you run the same tests locally?

@jewertow
Copy link
Contributor Author

This test fails when I run it locally due to:

docker pull footloose-alpine
Using default tag: latest
Error response from daemon: pull access denied for footloose-alpine, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

@jasmingacic
Copy link
Contributor

#788 fixed flakiness of the tests. Could you rebase your PR so this gets cleared up

@jewertow jewertow force-pushed the 743-dont-use-crictl-as-a-dependency branch from 89c547e to 0e3785d Compare March 22, 2021 11:52
@jewertow
Copy link
Contributor Author

@jasmingacic I rebased, but it failed once again. Could you rerun this step? As you saw before, it was working after a retry.

@jasmingacic
Copy link
Contributor

Ok it is passing now you have to fix the conflicting file :)

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Copy link
Contributor

@jasmingacic jasmingacic left a comment

Choose a reason for hiding this comment

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

The code is looking good. Once the conflict is resolved I will have it approved

@jewertow jewertow force-pushed the 743-dont-use-crictl-as-a-dependency branch from 0e3785d to f033184 Compare March 22, 2021 13:14
@jewertow
Copy link
Contributor Author

@jasmingacic I rebased PR and fixed conflicts. All checks have passed.

@jasmingacic
Copy link
Contributor

@trawler can you take a look too

Copy link
Contributor

@trawler trawler left a comment

Choose a reason for hiding this comment

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

lgtm! 👍

@jasmingacic jasmingacic merged commit 069c3ed into k0sproject:main Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k0s reset: don't use crictl as a dependency
3 participants