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

init: handle only "kudo-system" namespace #984

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

hypnoglow
Copy link
Contributor

What this PR does / why we need it:

Makes init command handle only kudo-system namespace. Other
namespaces are expected to be created beforehand.

Fixes #972

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I have just two small nits then it's ready to go.

Note: the failing e2e is our issue, we're working on it :)

secret := webhookSecret(opts)
var prereqs []runtime.Object

if opts.Namespace == defaultns {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this duplication (and it's not your fault!)

btw. If you want to follow up with another PR that makes Prereq be used in installPrereq that would be a good cleanup :) If not, I'll do that when this one merges

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, if you can provide me some tips: I'm not sure how to do this because installPrereq has only kubernetes.Interface client which operates on typed kubernetes objects, and Prereq returns a slice of untyped objects, which (as I saw in tests) should be managed by controller-runtime Client.

@hypnoglow hypnoglow force-pushed the do-not-handle-namespaces branch 2 times, most recently from 3c2198a to a7f4115 Compare October 22, 2019 21:08
Makes `init` command handle only `kudo-system` namespace. Other
namespaces are expected to be created beforehand.

Fixes kudobuilder#972
@hypnoglow hypnoglow marked this pull request as ready for review October 23, 2019 07:38
@alenkacz alenkacz merged commit 1600119 into kudobuilder:master Oct 23, 2019
@alenkacz
Copy link
Contributor

Thank you @hypnoglow !

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.

Init trying to create/delete default namespace
2 participants