-
Notifications
You must be signed in to change notification settings - Fork 389
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
Support loading into K3s with k3s.local #665
base: main
Are you sure you want to change the base?
Conversation
kameshsampath
commented
Mar 21, 2022
- use k3s.local domain
- use LIMA_INSTANCE as way to detect if its Rancher Desktop or vanilla lima-vm
- use k3s.local domain - use LIMA_INSTANCE as way to detect if its Rancher Desktop or vanilla lima-vm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good so far! I'm glad to see how (relatively) little work it is to get these images loaded into k3s!
I'd love to see some kind of e2e test, sort of like we have in kind-e2e.yaml -- to ensure this works, and that future changes don't break it. Is that something we could add relatively easily?
pkg/publish/k3s.go
Outdated
if err != nil { | ||
return nil, err | ||
} | ||
goos, goarch := os.Getenv("GOOS"), os.Getenv("GOARCH") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is largely copied from code in publish/kind.go
-- I think there's an opportunity to share this code instead of duplicating it, especially since I think there are improvements we could make to it later. Could you refactor this into some shared method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I thought so to refactor, no worries I could that now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@imjasonh - fixed this one with latest commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to see some kind of e2e test, sort of like we have in kind-e2e.yaml -- to ensure this works, and that future changes don't break it. Is that something we could add relatively easily?
I am exploring the ways to do it as installing Rancher Desktop or Lima requires the hypervisors on the runner and I am finding way to install/enable it. IMHO from my previous experiences I was not able to do it. 🤔
- refactor br image resolution to its own method - cleaned up k3s writer for review comments
@imjasonh tried doing a similar one https://github.com/kameshsampath/ko-demo-app/blob/main/.github/workflows/k3s-e2e.yaml but some reasons I am getting this error running the ko build
any clue ? |
Not really, honestly. 😕 Does this work when you try it on your own machine? Maybe there's some problem with k3s as it's set up by the action? FWIW, I can't imagine that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see anything obvious that might cause loading into k3s to fail. Just a few small comments in the meantime though.
Kind of got through the
IMHO - the runner is lacking in memory or CPU 🤔 . The same works very well in my local mac. |
It sounds like they should have plenty of resources to do a Just in case, it might make sense to lower the requirements specified here to match: https://github.com/kameshsampath/ko-demo-app/blob/main/ko-k3s-demos.yaml#L39-L49 The only one that exceeds the available runner resources is disk, maybe that's the problem? I'm sort of running out of ideas (if that's not obvious 😅), is there some Rancher Desktop expert we could tag in to help? Having a working Rancher Desktop CI setup seems useful in lots of other scenarios too, maybe we can borrow from some existing setup? |
yeah tried reducing the CPU to smallest possible values and trying . I am also checking with Rancher Desktop folks on their slack to see if they any thoughts 🤞🏾 |
@imjasonh Rancher is adding |
This Pull Request is stale because it has been open for 90 days with |