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

Refresh preview option #870

Open
michal-bajer1 opened this issue Feb 27, 2023 · 14 comments
Open

Refresh preview option #870

michal-bajer1 opened this issue Feb 27, 2023 · 14 comments
Assignees
Labels
area/cicd kind/enhancement Improvements or new features

Comments

@michal-bajer1
Copy link

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

I would like to build a Github workflow, which would regularly run preview of stack refresh and create an issue in the repository, if some changes between stack and our cloud provider are found.

Affected area/feature

The options of the refresh or preview command, so that the equivalent of pulumi preview --refresh can be executed.

@michal-bajer1 michal-bajer1 added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Feb 27, 2023
@dixler
Copy link
Contributor

dixler commented Feb 27, 2023

Hi. That's an interesting idea. I'm not too familiar with the actions side of things, but from https://github.com/pulumi/actions#configuration it seems like the following would accomplish what you're asking.

      - uses: pulumi/actions@v4
        with:
          command: preview
          refresh: true
        ...

If you need something more than that from the action, can you explain further what you might need?

@RobbieMcKinstry RobbieMcKinstry added area/cicd and removed needs-triage Needs attention from the triage team labels Feb 27, 2023
@michal-bajer1
Copy link
Author

Hi, there is a subtle difference there.
I tried this option, but what happens is that it runs refresh, stores the results into the stack and then it runs preview. But I don't really want the state of the stack to change unpredictably. What pulumi preview --refresh does is effectively preview of code vs cloud provider, but it doesn't change the state of the stack.

@peterstockwell-sagov
Copy link

Hello, we would also like an option to perform a preview with a refresh without updating the stack, although our use case is a little different. We run pulumi previews as part of our CI checks, however because the refresh option updates the stack, if multiple PRs are opened or updated at around the same time, one of the PR checks will fail as only one update can be performed on a stack at a time and requiring us to rerun it manually. Having the refresh run without updating the stack would solve this issue for us. We have raised this issue with @aureq privately already.

I saw @RobbieMcKinstry has commented in another issue about adding an option for raw as a command. I believe this would work for us and would be happy to implement and submit a PR with those changes if it helps.

@RobbieMcKinstry
Copy link
Contributor

Hi Peter (@peterstockwell-sagov),
Sorry for the slow response; we'd be happy to accept a PR for the raw command. :-)

if multiple PRs are opened or updated at around the same time, one of the PR checks will fail as only one update can be performed on a stack at a time

When I read this, my mind lept to GitHub Action's concurrency blocks. (Forgive me if you've already considered this.) At the granularity of jobs, you can block jobs from running at the same time if they're both PRs to the same base_ref, for instance.


I think @michal-bajer1 identified some very confusing behavior. It's confusing (to me) that…

- uses: pulumi/actions@v4
  command: preview
  refresh: true

…runs…

$ pulumi refresh; pulumi preview

instead of...

$ pulumi preview --refresh

If I had wanted the chaining behavior, I would have thought we'd chain the operations in the YAML instead:

- uses: pulumi/actions@v4
  command: refresh
- uses: pulumi/actions@v4
  command: preview

This is made even more confusing by pulumi/pulumi#10967

Unfortunately, I don't think we have a way to repair this behavior without redefining the behavior of refresh: true.
That leads me to believe @peterstockwell-sagov 's suggestion of adding a raw command would be the way forward here.

@michal-bajer1
Copy link
Author

@RobbieMcKinstry It definitely confused me at first :)

I guess you could try to introduce something like:

refresh-before-update: true - alias refresh: true
preview-including-refresh: true

But it isn't pretty :(

@RobbieMcKinstry
Copy link
Contributor

If I weren't worried about breaking changes, I would prefer to just drop the refresh-before-update behavior entirely, and suggest users use the @pulumi/action twice: once with command: refresh and once again with their intended behavior.

But you're right, that's an approach that would clarify the behavior without a breaking change :)

@simenandre
Copy link
Contributor

As there is no way to get the Automation API to run pulumi preview --refresh, we cannot do it with Automation API (which is why we did the weird pulumi refresh first).

Our intention was back then to do the --refresh argument precisely, but AFAIK, it still isn't available in Automation API.

I would love to see an idea for implementing raw command somehow smoothly. Might make sense to use the getExecOutput function that is included in @actions/exec and get the output that way. Let me know I can help!

@RobbieMcKinstry
Copy link
Contributor

As there is no way to get the Automation API to run pulumi preview --refresh, we cannot do it with Automation API (which is why we did the weird pulumi refresh first).

That makes sense! However, I think we could fix that problem ;D If we add the --refresh flag to the Automation API, then it sounds like we can resolve the issue by removing the refresh chaining and replacing it with the --refresh flag.

@RobbieMcKinstry
Copy link
Contributor

I opened pulumi/pulumi#12740 to address the missing flag in Automation API.

@RobbieMcKinstry RobbieMcKinstry self-assigned this Apr 25, 2023
bors bot added a commit to pulumi/pulumi that referenced this issue Apr 25, 2023
12743: Node AutoAPI: Support `preview --refresh` r=RobbieMcKinstry a=RobbieMcKinstry

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

This PR adds support for `pulumi preview --refresh` in Automation API for NodeJS.

Fixes #12740

This is a blocker for [actions/#870](pulumi/actions#870) and potential customer adoption.

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [x] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Robbie McKinstry <robbie@pulumi.com>
Co-authored-by: Robbie McKinstry <thesnowmancometh@gmail.com>
@arealmaas
Copy link

Any update to this? Would definitely like to see the same, and it seems like the pull request is merged now 🙏

@RobbieMcKinstry
Copy link
Contributor

Hi @arealmaas. Thanks for reaching out and expressing your interest ❤️
I'm no longer an OSS maintainer for this repository, so I'm afraid I'm no longer the point of contact for this issue. @simenandre (also from Oslo!) and the Pulumi Core team are currently leading the charge! 💪🏻

@simenandre
Copy link
Contributor

Hello 👋

I will have a look into this very soon! 🚀

@simenandre simenandre self-assigned this Nov 28, 2023
@fitz-vivodyne
Copy link
Contributor

Is this still on the roadmap/happening very soon?

@fitz-vivodyne
Copy link
Contributor

Took an initial stab at this in #1118 without knowing what I'm doing.

In a future major release it might make sense to change the behavior of the refresh option to use the --refresh flag, but for now I'm proposing a soft-refresh option in addition to refresh.

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

No branches or pull requests

7 participants