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

Fix pull policy #236

Merged
merged 2 commits into from
May 3, 2024
Merged

Fix pull policy #236

merged 2 commits into from
May 3, 2024

Conversation

nimakaviani
Copy link
Contributor

fixes #235

@greghaynes
Copy link
Contributor

Nice! We should probably open an issue to let users set this as I'm not sure there will be a way for a user to force all of these to update now, e.g. to pull in a new argo release.

@nimakaviani
Copy link
Contributor Author

it might be slightly easier to do this with sed in fact, given that we already do it to alter some other fields in the generated manifests.

Also to Greg's point, do we want to give the users the option to modify the pull policy on the fly? We dont do it for image references though.

@nabuskey
Copy link
Collaborator

nabuskey commented Apr 30, 2024

it might be slightly easier to do this with sed in fact, given that we already do it to alter some other fields in the generated manifests.

I'd rather stick to kustomize or helm features where possible. Direct manipulation through tools like sed should be for times where it's impossible for us to do it through helm or kusotmize.

Users can specify w/e manifest they want for core packages right now. We could add a feature just for images though since it's a very common task imo.

@blakeromano
Copy link
Contributor

I'd rather stick to kustomize or helm features where possible. Direct manipulation through tools like sed should be for times where it's impossible for us to do it through helm or kusotmize.

I am of the opinion we should be contributing upstream to add image pull policies rather then trying to do direct manipulation.

@nimakaviani
Copy link
Contributor Author

@blakeromano great point.

I am going to create an issue with Argo and possibly submit a PR for a fix to their helm chart. In the interim, to unblock us, let's get this merged, and we will do it the right way once Argo CD's helm chart has the ability to merge stuff.

@nimakaviani
Copy link
Contributor Author

asked a question on the argo cd slack.

Alternatively, rather than using the generated install manifest, we can use the argo cd helm chart and set the imagePullPolicy to the right value.

I will wait for a little longer to see if we get a response and if not, we can take the helm chart route.

@nimakaviani nimakaviani merged commit bd62b42 into main May 3, 2024
2 checks passed
@nabuskey nabuskey deleted the fix-pull-policy branch May 14, 2024 20:40
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.

Leverage local docker image cache
4 participants