helm namespaces #210
Merged
helm namespaces #210
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build Failed 😱 Build Id: 093d0f72-afa2-4f00-942f-52ee5f15f7e9 Build Logs |
|
Build Failed 😱 Build Id: 36dea8e3-3f8f-485b-8dc7-e2e42753a72c Build Logs |
|
Build Succeeded 👏 Build Id: b2ff6c0e-5bf3-492f-af75-144b2830958a The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
reviewed
May 18, 2018
| @@ -14,28 +14,32 @@ | |||
|
|
|||
| # Declare variables to be passed into your templates. | |||
|
|
|||
install/helm/README.md
Outdated
| $ helm install --set "gameservers.namespaces={default,xbox,ps4}"--name my-release agones | ||
| ``` | ||
|
|
||
| > You need to create your namespaces before installing agones. |
There was a problem hiding this comment.
Interesting question - should we create the namespaces for them? Because we could - and it's idempotent, so if they already exist, nothing is going to break. WDYT?
There was a problem hiding this comment.
I thought about this, wanted to ask on slack your opinion but it was late. I just need to make sure it doesn't block the deployment if the namespace exists.
There was a problem hiding this comment.
Yeah, good point - if it errors when the namespace already exists, then we shouldn't create the namespace - totally agreed.
Just thinking through some pros and cons otherwise:
Pros:
- Simpler install experience, since you just say what namespaces you want, and Helm will create them for you (if they don't already exist)
Cons:
- If you typo a namespace, you won't know it's broken, because the namespace will be created.
Can you think of any others? At least given the above, I'm leaning towards creating the namespaces, for the easier install experience. It's a bit more "turn key". WDYT?
There was a problem hiding this comment.
To capture our chat - keep things simple for now, and not implement this. Let's wait on feedback, as there is complexity with helm potentially owning the namespace.
There was a problem hiding this comment.
the best option if we really want this is to use a pre-install and pre-upgrade hook helm. The hook will create namespace if they don't exist so the namespace is not tracked by helm (hence not deleted when the release is deleted, could be dangerous), but that means the hook job needs permissions to create namespace, which complicate things, I also prefer to keep things simple and take action if a real need appears.
install/helm/README.md
Outdated
| @@ -27,6 +27,26 @@ The command deploys Agones on the Kubernetes cluster with the default configurat | |||
|
|
|||
| > **Tip**: List all releases using `helm list` | |||
|
|
|||
| ## Namespaces | |||
|
|
|||
| By default Agones is configured to work with game servers deployed in the default namespace. If you are planning to use other(s) namespace(s) you can configure Agones via the parameter `gameservers.namespaces`. | |||
There was a problem hiding this comment.
with game servers deployed in the
defaultnamespace
May want to add backticks around "default"
There was a problem hiding this comment.
If you are planning to use other namespaces
I'd suggest dropping the (s) on both, I don't think they are needed, and I feel like it reads a bit better.
|
Build Succeeded 👏 Build Id: c423ab49-1a86-49de-99e4-0ff2937cccb0 The following development artifacts have been built, and will exist for the next 30 days:
|
cyriltovena
force-pushed
the
feature/helm-gameservers-namespaces
branch
from
1077b79
to
8731f43
Compare
…ements in each namespace.
cyriltovena
force-pushed
the
feature/helm-gameservers-namespaces
branch
from
8731f43
to
6edd514
Compare
markmandel
approved these changes
May 18, 2018
|
Build Succeeded 👏 Build Id: ed753fa4-9b89-400e-b9ab-05f94e64bcc6 The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Succeeded 👏 Build Id: bf43bbdb-79d0-4c82-9e89-b5f3891b14fd The following development artifacts have been built, and will exist for the next 30 days:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
This fixes #146