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

idempotency test, particularly for Deployment Scripts #77

Closed
alex-frankel opened this issue Apr 14, 2022 · 13 comments
Closed

idempotency test, particularly for Deployment Scripts #77

alex-frankel opened this issue Apr 14, 2022 · 13 comments
Labels
Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue

Comments

@alex-frankel
Copy link

@shenglol I don't know if we already do this in our CI, but it would be good to validate idemopotency for a particular module. Technically this should be implied for any bicep code that deploys once, but we know that is not always the case.

For Deployment Scripts, idempotency in the script will not come for free. It would be good to ensure that these in particular are checked for idempotency.

@alex-frankel
Copy link
Author

FYI @bmoore-msft

@shenglol
Copy link
Collaborator

Good suggestion. I can modify our CI to repeat the deployment step for main.test.bicep.

@bmoore-msft
Copy link
Collaborator

The other part of it though will be ensuring the result is the same...

@shenglol
Copy link
Collaborator

The other part of it though will be ensuring the result is the same...

Agreed, but I don't if that's possible. What-If seems to be the right tool to use, but the results can be noisy for some RPs...

@bmoore-msft
Copy link
Collaborator

I think the goal of idempotency is that you get the same output with the same input... If we can't verify that, then not sure how much value the test will have - probably greater than zero, but doesn't tell you if it's a good script or not.

what-if won't tell you anything about the script part of the deployment script, or rather - what it's doing/changing.

@alex-frankel
Copy link
Author

We should be able to compare the results of script.properties.outputs on run one and run two, right? Brian, do you think that covers it?

@bmoore-msft
Copy link
Collaborator

that will certainly help - but what if the script just does stuff - and/or has no outputs? I think #62 just copied images. Granted that one is pretty easy to "check" but still has to be done manually...

I think all of the things we can do have some merit - just need to be aware of the cost/benefit and how much it can really tell us.

Another [semi-related] thing I just thought of (which probably also needs manual validation) is that if something can be done in the template, it must be done in the template, not in the script. And trying to avoid having too many scripts/modules that are working around issues in the platform (e.g. replication) - though admittedly I struggle with some of these...

@shenglol
Copy link
Collaborator

I thought we were talking about all resource types in general. If it is for deployment scripts only, comparing the outputs of the script requires the module author to export the script outputs as template level outputs of the test Bicep file. It also doesn't work if the outputs depend on some dynamic values (like timestamps).

@shenglol
Copy link
Collaborator

Brian also raised a good point...

@shenglol
Copy link
Collaborator

I wonder if we should leave the implementation of idempotency tests to the deployment-script module authors. What we can do is to ask them to provide Boolean outputs in the main.test.bicep file to assert scripts1.output.x == script2.outputs.x (if outputs.x should be idempotent). The CI can check if the outputs are true after deploying the test file.

@shenglol
Copy link
Collaborator

Another [semi-related] thing I just thought of (which probably also needs manual validation) is that if something can be done in the template, it must be done in the template, not in the script. And trying to avoid having too many scripts/modules that are working around issues in the platform (e.g. replication) - though admittedly I struggle with some of these...

The CI probably cannot help here, since checking if a script is doing something unnecessary requires domain knowledge. IMHO it must be done as part of the module proposal / code review process.

@bmoore-msft
Copy link
Collaborator

I wonder if we should leave the implementation of idempotency tests to the deployment-script module authors. What we can do is to ask them to provide Boolean outputs in the main.test.bicep file to assert scripts1.output.x == script2.outputs.x (if outputs.x should be idempotent). The CI can check if the outputs are true after deploying the test file.

I like this idea (and second your point about doing this for all RPs not just scripts)... Though I also wonder if we should just leave it to a manual review - given we'd have to for the domain knowledge of the scenario (i.e. is this possible in a template without a script) we could do it for idempotency as well... I think it's at least worth trying manually - see how many we get, how hard the problem is, before we try to automate it...

@ghost ghost added the Needs: Triage 🔍 Maintainers need to triage still label Apr 20, 2022
@shenglol shenglol removed the Needs: Triage 🔍 Maintainers need to triage still label Apr 20, 2022
@ChrisSidebotham
Copy link
Contributor

Dear @bmoore-msft , Azure Verified Modules (AVM) will become the single Microsoft standard for Bicep modules. Transitioning to AVM, requests for new features or bug fixes will need to be submitted against the appropriate AVM module via AVM Module Issues.

For bugs or new features with existing modules in the /modules directory of this repository, we encourage you to first try the AVM equivalent module if available and if the module is lacking any features, you can file an AVM Module Issue. If the module is not available yet, you can file a new AVM Module Proposal.

See this informational notice for more details on the upcoming changes.

Thank you for your understanding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue
Projects
None yet
Development

No branches or pull requests

4 participants