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

feat: optionally allow users to install pod restarts check with a clu… #897

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

rawlingsj
Copy link
Contributor

…ster scope

similar to this PR #681

- env:
- name: MAX_FAILURES_ALLOWED
value: "10"
image: kuberhealthy/pod-restarts-check:v2.3.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I need to do anything with this tag? As this needs to points to a version that hasn't been released yet so maybe the release pipeline updates this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is actually manual... This is basically documentation. The versions in the helm chart (and the flat files in the directory with the helm chart) get updated after we change the version.yaml of the chart, but this one does not.

@@ -192,6 +192,7 @@ check:
registry: kuberhealthy
repository: pod-restarts-check
tag: v2.3.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above, I'm not sure if I need to do anything with this tag? As this needs to points to a version that hasn't been released yet so maybe the release pipeline updates this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this version v2.3.3 has been released? https://hub.docker.com/repository/registry-1.docker.io/kuberhealthy/pod-restarts-check/tags?page=1&ordering=last_updated

hmm we don't have the best release process for specific versions for these external checks -- when a pr for external checks get merged in we do have a github action in place for build and pushing "latest". For specific versions, we build and push manually :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok cool - makes sense, so is it best to change that to latest in this PR and let the followup PR set the correct tag?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep... this is the flow I would expect:

  • third party check makes a new release version
  • third party check opens a PR to edit this values.yaml file for the helm chart
  • helm chart static files are regenerated automatically

@rawlingsj rawlingsj force-pushed the master branch 2 times, most recently from 8530d00 to 3056bfc Compare April 27, 2021 06:15
rawlingsj added a commit to rawlingsj/jx-kh-check that referenced this pull request Apr 27, 2021
…tream PR to be merged

we can remove this check once upstream PR is merged kuberhealthy/kuberhealthy#897
rawlingsj added a commit to rawlingsj/jx-kh-check that referenced this pull request Apr 27, 2021
…tream PR to be merged

we can remove this check once upstream PR is merged kuberhealthy/kuberhealthy#897
rawlingsj added a commit to rawlingsj/jx-kh-check that referenced this pull request Apr 27, 2021
…tream PR to be merged

we can remove this check once upstream PR is merged kuberhealthy/kuberhealthy#897
@joshulyne
Copy link
Collaborator

@rawlingsj thanks so much for contributing this! will check it out and test it soon!

Copy link
Collaborator

@joshulyne joshulyne left a comment

Choose a reason for hiding this comment

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

lgtm!

@joshulyne joshulyne merged commit 104389c into kuberhealthy:master Apr 28, 2021
@joshulyne
Copy link
Collaborator

@rawlingsj upped the pod-restarts-check to v2.4.0 which should have this clusterScope feature!

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.

None yet

3 participants