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

Re-consider --dry-run=server for previews #3073

Open
blampe opened this issue Jun 25, 2024 · 3 comments
Open

Re-consider --dry-run=server for previews #3073

blampe opened this issue Jun 25, 2024 · 3 comments
Labels
kind/enhancement Improvements or new features

Comments

@blampe
Copy link
Contributor

blampe commented Jun 25, 2024

Placeholder for v5 wishlist.

Server-side dry-run has a number of surprising edge cases, and we might want to consider client-side previews for v5. Keeping in mind that a lot of users rely on previews as a CI test, so false positives (where the preview succeeds but update fails) can be very disruptive.

#3053 (review)

@blampe blampe added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Jun 25, 2024
@rquitales rquitales removed the needs-triage Needs attention from the triage team label Jun 26, 2024
@rquitales
Copy link
Contributor

We may also want to reconsider this for Helm chart templating (#3247).

@blampe
Copy link
Contributor Author

blampe commented Oct 8, 2024

We may also want to reconsider this for Helm chart templating (#3247).

It's interesting because the Helm docs seem to recommend server-side apply when previewing charts that use this lookup method. It sounds like it might not even work client-side?

Keep in mind that Helm is not supposed to contact the Kubernetes API Server during a helm template|install|upgrade|delete|rollback --dry-run operation. To test lookup against a running cluster, helm template|install|upgrade|delete|rollback --dry-run=server should be used instead to allow cluster connection.

@EronWright
Copy link
Contributor

EronWright commented Oct 14, 2024

@blampe note that Chart v4 uses server-side dry-run, to support the lookup method. Lookup is similar to a Pulumi resource get method, in that it assumes the object exists even during preview. I don't see an issue with reading from the cluster during preview, of course mutations are to be avoided.

Another area that would be affected is the CSA-to-SSA migration logic. I observe that the provider skips doing migration during preview, but then may see a conflict message (during preview, not in the update). See thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

3 participants