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

Adds optional addons for microk8s #46

Merged
merged 6 commits into from
Dec 13, 2022
Merged

Adds optional addons for microk8s #46

merged 6 commits into from
Dec 13, 2022

Conversation

ca-scribner
Copy link
Contributor

This PR replaces #45, which itself extended #33. The PR originates from @kian99 who does not have permissions in this repo, so the full CI suite cannot be executed. I'm opening this clone of @kian99's PR so we can get the full test suite.

Copied from #45:

In order to be able to use the nginx-ingress-integrator charm and other related charms, we need to enable ingress in microk8s.

Copied from #33:
This adds the microk8s-addons option, which will contain the addons to be enabled in microk8s. If not given, the default addons will be added instead (storage, dns, rbac).

This PR can be merged in favor of the original, it simply adds the original changes on top of the latest commit in main. An example use can be found here with corresponding workflow file.

Fixes #32

claudiubelu and others added 3 commits November 24, 2022 11:32
In order to be able to use the nginx-ingress-integrator charm and other
related charms, we need to enable ingress in microk8s.

This adds the microk8s-addons option, which will contain the addons to be
enabled in microk8s. If not given, the default addons will be added instead
(storage, dns, rbac).
- Updated Readme with more info
- Added extra tests
@ca-scribner
Copy link
Contributor Author

#45 includes a few conversations about design decisions, so reviewing the conversations there will help with understanding this PR.

@kian99
Copy link
Contributor

kian99 commented Dec 1, 2022

Seems like the test failed for 2 separate reasons - the first test failed because the call microk8s status needs to be made with sudo. Then the test with the empty addons failed because part of the init script deploys coredns to the cluster and without the dns addon it fails. After deploying coredns it also deploy hostpath-provisioner and creates a service account so I suspect those would also fail without the other default addons. I'm thinking either we optionally deploy those services or add note in the readme that those addons must be enabled. Thoughts?

@ca-scribner
Copy link
Contributor Author

oh interesting, so in (most?) cases doing addons="" is useless anyhow. darn haha. Ok let me post some fixes

- This implies that other addons may be enabled by default.
- Updated README to indicate that a minimum set of addons are required.
Copy link
Member

@addyess addyess left a comment

Choose a reason for hiding this comment

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

only minor security concerns. Otherwise LGTM

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

Successfully merging this pull request may close these issues.

Enable ingress on microk8s
4 participants