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

CleanCmd that saves *ItemState::None causes Apply out-of-sync error on subsequent invocation #188

Open
azriel91 opened this issue Mar 6, 2024 · 0 comments

Comments

@azriel91
Copy link
Owner

azriel91 commented Mar 6, 2024

There is a confusing case with StatesCurrent and deploying.

When we have:

  1. an S3Bucket and an S3Object
  2. the S3Object has a parameter built from S3Bucket

Then:

  1. The initial state from DiscoverCmd is null (since the parameter is unknown).
  2. After deploying and cleaning, S3Object's state is None, because when we clean it up, we know it is None.
  3. When EnsureCmd is run, the discovered value is null (unknown).
  4. This is detected as out-of-sync.
./envman deploy --fast
# Errors

peace_rt_model::apply_cmd_error::states_current_out_of_sync

  × Stored current states were not up to date with actual current states.
  │
  │ * s3_object:
  │
  │     - stored: does not exist
  │     - discovered: <none>
  │
  │
  help: Run `StatesDiscoverCmd::current` to update the stored current states,
        and re-check the difference before applying changes.

Solutions

Option A: We should not consider null vs SomeItemState::None as out-of-sync when deploying?

  • 🟢 Usually null (unknown) is alright to tolerate ItemState::None on discovery on ensure.
  • 🔴 If the item actually exists, maybe we should stop, maybe we should "just use it".

Option B: We save states as null after cleaning up.

  • 🔴 Doesn't work with the other items that need to be non-null when deploying next.

Option C: Discover states again after cleaning

  • 🟢 Ensures consistency with the States that will be discovered upon the next deploy
  • 🔴 Adds another discovery duration to the current execution.
azriel91 added a commit that referenced this issue Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

1 participant