add Preflight Validate API support #4329
Conversation
To assign the process to the job object after we create it, we need the `windows.Handle` that backs the process we started with `os/exec`. This was not exported and so we used some gnarly casting to cast the `os.Process` to a structure we authored that had the same layout of `os.Process` so we could grab the handle property. This is unsafe if the layout of the `os.Process` changes, which it did in Go 1.23. Move to code that doesn't depend on the internals of `os.Process` by using the exported Pid to call `windows.OpenProcess` to get a handle and then pass that along to `windows.AssignProcessToJobObject`.
hemarina
requested review from
ellismg,
wbreza,
vhvb1989 and
weikanglim
as code owners
| err = p.validatePreflight( | ||
| ctx, | ||
| bicepDeploymentData.Target, | ||
| bicepDeploymentData.CompiledBicep.RawArmTemplate, | ||
| bicepDeploymentData.CompiledBicep.Parameters, | ||
| deploymentTags, | ||
| ) |
There was a problem hiding this comment.
I think it is great that the validate API exists but I am concerned that this may cause considerable time delay during deployments.
Do we have an idea of the amount of extra time the validations will take especially against more complex & nested deployments?
There was a problem hiding this comment.
@hemarina and I had spoke awhile back about this while she was implementing -- I think this is pretty lightweight (<3s) from what she showed me, but it'd be great to test this out and gather the numbers with a few different templates.
There was a problem hiding this comment.
It is lightweight as Wei said but I'll run some tests to confirm the exact number or ask manual test team to run all todo templates.
There was a problem hiding this comment.
What is the expectation for templates with nested deployments? For example, I used our Todo AKS template, specified an invalid K8s version parameter and expected that this would fail at our own preflight check call.
Instead the preflight from azd passes, but then we get a preflight failure during ARM deployment.
Preflight validation check for resource(s) for container service aks-oc3hr2l7nqlee in resource group rg-wabrez-todo-aks-swed failed. Message: Web application routing requires at least Kubernetes 1.22. Details:
(Code: WebAppRoutingUnsupportedK8SVersion)
If it is not going to catch preflight errors on nested deployments I question whether it is valuable to add at this point.
| var rawResponse *http.Response | ||
| ctxWithResp := runtime.WithCaptureResponse(ctx, &rawResponse) |
There was a problem hiding this comment.
Do we need to do this? If we get an HttpError we should be able to case the error to ARM http error to parse the error message.
There was a problem hiding this comment.
It's a workaround for a related bug here: Azure/azure-sdk-for-go#23350
|
|
||
| _, err = validate.PollUntilDone(ctx, nil) | ||
| if err != nil { | ||
| deploymentError := createDeploymentError(err) |
There was a problem hiding this comment.
We should use this same createDeploymentError() func in the deployment stack use cases as well.
| type PreflightErrorResponse struct { | ||
| Error struct { | ||
| Code string `json:"code"` | ||
| Message string `json:"message"` | ||
| Details []struct { | ||
| Code string `json:"code"` | ||
| Message string `json:"message"` | ||
| } `json:"details"` | ||
| } `json:"error"` | ||
| } |
There was a problem hiding this comment.
It looks like this basically captures the same ARM errors that we use in the AzureDeploymentError object that we already have. Can we reuse that one instead?
Adds support for Preflight Validate API.
the operation failed or was cancelledneeds detailed message azure-sdk-for-go#23350TODO:
Questions
What is Preflight?
Preflight API validates whether the specified template is syntactically correct and will be accepted by Azure Resource Manager. More information on preflight wiki.
Why do we implement Validate API?
There’re two preflight API: Validate API and Deployment API. We implemented deployment API. However, deployment API creates a scenario where specific resource fails at preflight API and stops its deployment where some resources are already created. For example, resources are created at an unsupported location. This will cause more dev-test loop for cleaning resources and re-deploy. Adding Validate API support before provisioning to azure solves this situation.
What are Preflight Validate API limitations?
Example Test Error Message
Standard deployments
Deployment stack