Add crd-install helm hook to crds templates #375
Merged
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: bb686f3e-7b6e-4c72-9114-e41525cd2b9c Build Logs |
|
You will want to run |
|
Build Failed 😱 Build Id: 3d6ed650-d765-42e8-9654-600c3600de0c Build Logs |
|
I am not sure about the reason of the test error. Adding annotations should not make the installation of helm charts to be broken. |
|
Ah! I was just looking at this - we are on helm version 2.9.1. I don't think there is any reason we can't upgrade - just need to do the work. Separate PR would be lovely 💃 |
markmandel
added
feature-freeze-do-not-merge
markmandel
removed
feature-freeze-do-not-merge
markmandel
added a commit
to markmandel/agones
that referenced
this pull request
Oct 23, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable googleforgames#375 to pass tests as well.
Merged
markmandel
added a commit
to markmandel/agones
that referenced
this pull request
Oct 25, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable googleforgames#375 to pass tests as well.
markmandel
added a commit
to markmandel/agones
that referenced
this pull request
Oct 30, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable googleforgames#375 to pass tests as well.
markmandel
added a commit
to markmandel/agones
that referenced
this pull request
Oct 30, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable googleforgames#375 to pass tests as well.
markmandel
added a commit
to markmandel/agones
that referenced
this pull request
Oct 30, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable googleforgames#375 to pass tests as well.
markmandel
added a commit
that referenced
this pull request
Oct 30, 2018
This will likely fail on first PR, as we will need to update the Helm install on the e2e cluster. A good opportunity to delete and restart the e2e cluster anyway -- but will need to manage this around other PRs. Should enable #375 to pass tests as well.
|
Helm upgrade complete. Let's try this again! |
|
Build Failed 😱 Build Id: f7fa2906-5bd6-466a-aa33-993e2b4b3b7d Build Logs |
|
@Kuqd any thoughts on below 😢 Did the consul lock not exit somehow? I'm wondering if we got a new version of consul with the new cluster? |
|
Build Failed 😱 Build Id: 593e1ec7-12d5-4cbd-8806-8736d46f1bb5 Build Logs |
|
We've investigated, and these changes make the helm chart fail. It's not the e2e test. |
markmandel
requested changes
Oct 31, 2018
| @@ -22,6 +22,8 @@ metadata: | |||
| chart: {{ template "agones.chart" . }} | |||
| release: {{ .Release.Name }} | |||
| heritage: {{ .Release.Service }} | |||
| annotations: | |||
| "helm.sh/hook": crd-install | |||
There was a problem hiding this comment.
Suggested change
| "helm.sh/hook": crd-install | |
| "helm.sh/hook": "crd-install" |
| @@ -22,6 +22,8 @@ metadata: | |||
| chart: {{ template "agones.chart" . }} | |||
| release: {{ .Release.Name }} | |||
| heritage: {{ .Release.Service }} | |||
| annotations: | |||
| "helm.sh/hook": crd-install | |||
There was a problem hiding this comment.
Suggested change
| "helm.sh/hook": crd-install | |
| "helm.sh/hook": "crd-install" |
| @@ -22,6 +22,8 @@ metadata: | |||
| chart: {{ template "agones.chart" . }} | |||
| release: {{ .Release.Name }} | |||
| heritage: {{ .Release.Service }} | |||
| annotations: | |||
| "helm.sh/hook": crd-install | |||
There was a problem hiding this comment.
Suggested change
| "helm.sh/hook": crd-install | |
| "helm.sh/hook": "crd-install" |
| @@ -22,6 +22,8 @@ metadata: | |||
| chart: {{ template "agones.chart" . }} | |||
| release: {{ .Release.Name }} | |||
| heritage: {{ .Release.Service }} | |||
| annotations: | |||
| "helm.sh/hook": crd-install | |||
There was a problem hiding this comment.
Suggested change
| "helm.sh/hook": crd-install | |
| "helm.sh/hook": "crd-install" |
| @@ -22,6 +22,8 @@ metadata: | |||
| chart: {{ template "agones.chart" . }} | |||
| release: {{ .Release.Name }} | |||
| heritage: {{ .Release.Service }} | |||
| annotations: | |||
| "helm.sh/hook": crd-install | |||
There was a problem hiding this comment.
Suggested change
| "helm.sh/hook": crd-install | |
| "helm.sh/hook": "crd-install" |
| @@ -163,6 +163,8 @@ metadata: | |||
| chart: agones-0.6.0-rc | |||
| release: agones-manual | |||
| heritage: Tiller | |||
| annotations: | |||
There was a problem hiding this comment.
Then this needs to be regenerated.
|
Build Failed 😱 Build Id: 4cd0fd72-f7fa-42de-8cd5-9ee6eb049e60 Build Logs |
|
Build Failed 😱 Build Id: 6c0c294b-7acc-4263-a3eb-f214a953d337 Build Logs |
|
This keeps failing. Happy to dig in some more, but wanted to check - it's this working on your end? Wondering if there is some difference in setup here that I'm missing. |
markmandel
approved these changes
Nov 11, 2018
markmandel
requested changes
Nov 11, 2018
There was a problem hiding this comment.
I approved this so that I could try and update the branch, but doesn't look like it's going to let me.
I rebased against master - and then it seemed to work fine locally. Can you try that at your end and push it up. Not sure why github isn't letting me. Still curious if this is this is working for you locally?
I'll retry a build now.
Failing that, do you mind if I take over the PR?
|
Running another test - see if it passes. Also, was wrong about the rebase. That's weird. Not sure what is up. |
|
Build Failed 😱 Build Id: 421c8f80-de52-4496-8d85-afc692ac4550 Build Logs |
|
Build Failed 😱 Build Id: 2fdb4a71-18e5-4290-a66a-8167a74ef349 Build Logs |
|
Build Failed 😱 Build Id: 9c3d688f-d986-4384-a864-53f7796cb138 Build Logs |
|
Build Failed 😱 Build Id: aa529ce4-56e1-42a7-b107-8be7a6a3a8ff Build Logs |
|
Build Succeeded 👏 Build Id: 82bfb744-a6f2-4d20-8cf8-92f55d4837d4 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
|
Build Succeeded 👏 Build Id: a561adc8-9526-439d-adf9-d4fbbe210966 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
markmandel
approved these changes
Nov 13, 2018
|
Thanks for managing this! |
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.
From Helm documentation:
This PR aims to update agones helm charts crds adding the
crd-installhook, ensuring then the installation of those to be done before other workloads.This change also fixes third party charts with agones as dependency that deploy workloads related to those custom resources.