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

Kustomise files for daily #256

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Kustomise files for daily #256

merged 1 commit into from
Nov 23, 2021

Conversation

idlewis
Copy link
Collaborator

@idlewis idlewis commented Nov 11, 2021

start to deal with #177

Copy link
Member

@leochr leochr left a comment

Choose a reason for hiding this comment

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

@idlewis Thanks. Looks good. Just want to confirm the purpose of all-namespaces2. Is it to specify a different namespace? Wondering whether changing namespace: default in all-namespaces/kustomization.yaml would achieve the same

@idlewis
Copy link
Collaborator Author

idlewis commented Nov 16, 2021

@leochr Yes, the purpose of the all-namespaces2 is to specify a different a namespace, and yes this could be done in all-namespaces. However, the idea was that, because all-namespaces adds in a new resource file (the cluster roles yaml) I was expecting that users would want to treat all-namespaces as a base to build on, and probably would prefer to leave it untouched, rather than having to edit it. So all-namespaces2 is an example of how a user could build on all-namespaces without having to modify it.
If you think it is better to combine the two, I'm happy to do that.
I also will add a readme to explain it either way, once you are happy with the content.

@leochr
Copy link
Member

leochr commented Nov 16, 2021

Thanks @idlewis . It'll be good to separate the folders that customers should modify from the folders/files we provide (i.e. move all-namespace2 and test-namespace under examples)

Suggest the following names for the folders:
watch-own-namespace
watch-another-namespace
watch-all-namespaces

Copy link
Member

@leochr leochr left a comment

Choose a reason for hiding this comment

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

@idlewis Thanks for the update. If possible, it'll be good to have the examples directly under daily folder. This way that's the only folder customers would have to edit and also avoid accidentally modifying other folders directly under overlays.

Maybe you are planning to add, it'll be good to have an example for watch-another-namespace.

@idlewis
Copy link
Collaborator Author

idlewis commented Nov 17, 2021

@leochr I think I've addressed all the review comments. Hopefully should be ready for review/merge.

@halim-lee halim-lee mentioned this pull request Nov 23, 2021
2 tasks
Copy link
Member

@leochr leochr left a comment

Choose a reason for hiding this comment

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

@idlewis Looks good. Thanks.

@leochr leochr merged commit 105f738 into master Nov 23, 2021
@idlewis idlewis deleted the kustom4 branch November 25, 2021 10:27
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.

2 participants