-
Notifications
You must be signed in to change notification settings - Fork 8
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 microk8s controller #1216
Add microk8s controller #1216
Conversation
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
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 is great, thanks a bunch! Just some comments before I approve.
@@ -0,0 +1,13 @@ | |||
#!/bin/bash | |||
|
|||
# Host-access has some issues, TLDR to fix it: |
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.
could this be automated? like.. adding microk8s snap install to makefile target dev-env-setup along with enable storage and host-access..
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'm a bit hesitant on this idk, we could put it in make target??
@@ -4,15 +4,13 @@ | |||
# It will bootstrap a Juju controller and configure the necessary config to enable the controller |
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.
suggestion.. should this be renamed to setup-lxd-controller.sh to keep the naming convention in line with setup-microk8s-controller.sh
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 can do, but you won't see this when running top level scripts
e705e13
to
c0fca0e
Compare
396ce2f
to
d1f4528
Compare
qa-microk8s.sh
Outdated
@@ -0,0 +1,40 @@ | |||
#!/bin/bash |
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.
agree with @kian99 .. both qa scripts should live in ./local/
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.
lgtm, just curious if the microk8s controller works?
It does, me and babak added a model and deploy hello-kubecon |
…k8s-controller
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.
All good, only one small comment
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.
Maybe cleanup() could be extracted to a separate file and sourced to keep it DRY since the only difference between the fn in qa-lxd and qa-microk8s is the name passed to juju destroy-controller? If some changes in the future affect this it's always nicer to have to bring only one script up-to-date.
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 agree, but just want to land this as its been waiting ages, can come back and fix in later?
Enables local qa setup for microk8s without the hassle of doing 100000000 steps.