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

Handle WhatIf #70

Open
SteveL-MSFT opened this issue Apr 24, 2023 · 14 comments · May be fixed by #400
Open

Handle WhatIf #70

SteveL-MSFT opened this issue Apr 24, 2023 · 14 comments · May be fixed by #400
Assignees
Labels
Issue-Enhancement The issue is a feature or idea
Milestone

Comments

@SteveL-MSFT
Copy link
Member

Summary of the new feature / enhancement

Need to have resources support this to work completely

Proposed technical implementation details (optional)

No response

@SteveL-MSFT SteveL-MSFT added the Issue-Enhancement The issue is a feature or idea label Apr 24, 2023
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Apr 24, 2023

Maybe it's just a special arg defined for set. Don't think we need it for get and test

@Bpoe
Copy link
Collaborator

Bpoe commented May 8, 2023

How would WhatIf be different than Test?

@SteveL-MSFT
Copy link
Member Author

Maybe it's implemented via Test?

@mgreenegit
Copy link
Member

I would see them as distinct.

A test operation on a file copy evaluates if the files at the destination match the files at the source.

A set --whatif operation would run the set script with the cmdlet bindings so the output would log everything that would happen if set were actually running, kind of like a simulation.

@SteveL-MSFT
Copy link
Member Author

@mgreenegit we need to think how this would be implemented and what result would be returned. If we require resources to implement it, then it's less likely to be supported. Ideally, we can extract it from the resource metadata in the manifest. A dsc config set --whatif could just do a test and return only what changes would apply rather than test today returning what is different. Perhaps additional optional metadata in the manifest could indicate more destructive operations (aka ShouldProcess) although in the case of the registry, for example, any overwrite would be considered destructive.

@Bpoe
Copy link
Collaborator

Bpoe commented Jul 5, 2023

I'm not sure how this is different than Test. Is this something that exists in DSC today?

@michaeltlombardi
Copy link
Collaborator

While Invoke-DscResource doesn't have support for the WhatIf parameter in any version I have installed locally, the Start-DscConfiguration cmdlet in Windows PowerShell 5.1 does.

@michaeltlombardi
Copy link
Collaborator

I think in earlier versions of DSC the WhatIf parameter served a useful purpose, because the only information we got back from the Test method was boolean true/false. In that model, running with WhatIf was the only way to get a sense of how your system would change without actually applying the changes and potentially changing system state.

In the new model, where Test returns more useful information, like:

expected_state:
  scope: machine
  ensure: present
  updateAutomatically: false
actual_state:
  ensure: absent
  scope: machine
diff_properties:
- ensure
- updateAutomatically

In this new model, WhatIf support is primarily a different view of the same data, which can be useful for reporting and other integrating tools that have conceptual support for WhatIf (for example, Puppet has a --noop flag that does the same sort of thing).

Reusing the output from above, that might look something like (keywords/structure just examples, not proposals):

changes:
  ensure:
    from: present
    to:   absent

@mgreenegit
Copy link
Member

Agree with @michaeltlombardi

I'm a little concerned it will be confused with PowerShell whatif. Maybe we should call it by a different name?

@dkattan
Copy link

dkattan commented Aug 17, 2023

I think there should be a separate stream with WhatIfMessages and in the absence of a Test implementation and Set implements SupportsShouldProcess, Test would call Set with -WhatIf, capture the WhatIfMessages and if any exist (indicating that something needs to get done) then return false.

I also think it would be nice if WhatIfMessages themselves could contain a compliant boolean so that as the set goes through what it would do, you can get positive compliance messages instead of the presence of a WhatIf message indicating non-compliance but not indicating to the user what items are compliant.

My experience is that Test and Set generally share a lot of code and look similar. Doing this would prevent the need for code duplication in the Test.

@michaeltlombardi
Copy link
Collaborator

@dkattan trying to rephrase your comment to avoid PowerShell-specific implementation details:

  1. Resources should be able to emit structured messages that indicate what a resource would change during a set operation.
  2. Those messages should describe the change, like setting property 'foo' from 'bar' to 'baz', and include boolean value where true indicates that the property is already in the desired state and false indicates that it isn't.
  3. That those messages in set running in this non-apply mode (what-if, noop, whatever it gets called) could be an alternative to relying on DSC's synthetic test, where any false value in a message indicates that the resource instance is out of the desired state.

My experience is that Test and Set generally share a lot of code and look similar. Doing this would prevent the need for code duplication in the Test.

In v3 the implementation for the test operation is only required when the resource's properties can't be evaluated with a case-insensitive equivalency check (DSC's built-in synthetic test). If a resource doesn't define test, DSC handles it for them.

Specifically regarding the code duplication problem, I've updated the PowerShell documentation for authoring class-based resources such that the Test() method always calls Get() and the Set() method always calls Test(). The duplication of code across get/set/test is an anti-pattern. The new tutorial for implementing a command-based DSC Resource in Go doesn't implement test at all, but does include logic in the set command for only changing state if changes need to be made.

@SteveL-MSFT
Copy link
Member Author

Based on most recent discussion, we still need to define schema for whatif output, but we have a mechanism to tell supported resources to emit whatif #218 (comment). For resources that don't support it, dsc will have a generic message on calling the resources with some non-sensitive parameters

@michaeltlombardi
Copy link
Collaborator

I'm a little concerned it will be confused with PowerShell whatif. Maybe we should call it by a different name?

Alternatives used by other tools include:

  • Puppet as a noop mode, set with the --noop flag or noop metaparameter in an instance definition.
  • Ansible has a check mode, set with the --check flag or check_mode in a playbook.
  • Chef has a why-run mode, set with the --why-run flag, though Chef recommends not using it, as described in their blog post Why "Why-Run" Mode Is Considered Harmful by Julian Dunn - essentially, that it isn't an effective tool for validating what a run will do or a substitute for compliance validation.
  • Salt has both a test and mock state that don't execute changes.
  • Terraform has a plan command to show what an apply will do.
  • Pulumi has a preview command to show what an up will do.

Of these options, I think the semantics of preview are most aligned with whatIf - though it's worth noting that ARM does have the what-if functionality itself, so reusing that semantic seems okay to me. I do like the combination of having both a --what-if flag and --confirm flag, the latter showing the what-if output per-resource and prompting for a continuation.

It might be worth defining both whatIf and confirm boolean options for the resource, borrowing from Puppet and Ansible to have a per-resource alternate behavior:

resources:
  - name:   Only show what-if
    type:   Microsoft.Windows/Registry
    whatIf: true
    properties:
      keyPath:    HKCU\tailspin\updates
      valueName:  automatic
      valuData: { String: enable }
  - name:    Show what-if, prompt to enforce
    type:    Microsoft.Windows/Registry
    confirm: true
    properties:
      keyPath:    HKCU\tailspin\updates
      valueName:  automatic
      valuData: { String: enable }

I would expect:

  • confirm: true to supercede whatif: false
  • Both to default to false for an instance declaration
  • --confirm and --what-if to apply to every instance - probably overriding any explicit confirm: false or whatIf: false settings for instances.

@SteveL-MSFT
Copy link
Member Author

For now, we can focus on whatif and leave confirm as a separate feature.

For the implementation, we have a Context struct which can be reused for this. Add a member to track whatif execution and pass it along so that each invocation knows to emit info rather than perform actual execution.

We can also defer resource participation as I think it will depend on #218 which will expose a pattern to pass optional args to exes to indicate whatif.

@tgauth tgauth linked a pull request Apr 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement The issue is a feature or idea
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants