-
Notifications
You must be signed in to change notification settings - Fork 689
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
Remove the hack in scripts for certgen job #4402
Conversation
Signed-off-by: Rajat Vig <rvig@etsy.com>
Codecov Report
@@ Coverage Diff @@
## main #4402 +/- ##
==========================================
- Coverage 78.34% 78.33% -0.01%
==========================================
Files 113 113
Lines 10196 10196
==========================================
- Hits 7988 7987 -1
- Misses 2025 2026 +1
Partials 183 183
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thanks!
@youngnick Would you know why the build failed for release notes? Do not see any options to add release note labels. |
@@ -4908,7 +4908,7 @@ spec: | |||
containers: | |||
- name: contour | |||
image: ghcr.io/projectcontour/contour:main | |||
imagePullPolicy: Always | |||
imagePullPolicy: IfNotPresent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was set this way on the main
branch because if not you'd never get an updated version of it. The versioned examples do change this to Always
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevesloka I think you're suggesting that we move this logic out of this script that generates the rendered manifests on all branches, and into https://github.com/projectcontour/contour/blob/main/hack/release/make-release-tag.sh so it only applies to rendered manifests on release branches?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess my point was on the main
branch, if you used this code (which you really shouldn't but only in a dev/test env), then the 2nd time you updated you wouldn't get the new version of Contour since there's a version already present on the machine. =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep makes sense -- this has been true for the contour serve, bootstrap and shutdown manager image refs for a long time so odds are nobody's actually using this manifest anyway, but filed #4404 to track the additional change
examples/render: Set the
imagePullPolicy
for certen job toIfNotPresent
Set the
imagePullPolicy
for certen job toIfNotPresent
Fixes #4318
Signed-off-by: Rajat Vig rvig@etsy.com