Skip to content

Commit e5cd73b

Browse files
jongioCopilot
andauthored
fix: gracefully handle missing AKS cluster during postprovision hook (Azure#7501)
* fix: gracefully handle missing AKS cluster during postprovision hook When infrastructure doesn't include AKS resources, the postprovision lifecycle hook in the AKS service target fails fatally trying to set up the Kubernetes context. This is expected in multi-phase provisioning workflows where AKS gets provisioned later. Modified setK8sContext() to detect the postprovision event and skip gracefully (with a user-visible warning) instead of failing when: - GetTargetResource returns an error (resource not found) - Target resource has an empty name (SupportsDelayedProvisioning) - Cluster credentials are unavailable (ensureClusterContext fails) - Namespace creation fails (ensureNamespace fails) The predeploy event path is unchanged and still fails fatally, ensuring deployment-time errors are not masked. Extracted skipPostprovisionK8sSetup() helper to eliminate repeated warning/log/return-nil pattern across all four skip points. Fixes Azure#3272 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * address review: propagate ctx cancellation, add namespace failure test - skipPostprovisionK8sSetup now checks ctx.Err() before returning nil to avoid swallowing context cancellation/timeouts (Ctrl+C) - Added Test_Postprovision_Skips_When_Namespace_Fails covering the ensureNamespace failure path during postprovision Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: address MQ review — typed constant, DRY helper, ctx cancellation test - Replace magic string 'postprovision' with typed postProvisionEvent constant - Extract postprovisionK8sError() helper to eliminate 4 identical if-blocks - Add nil-guard on targetResource before calling ResourceName() - Fix log.Printf double newline and redundant .Error() call - Add Test_Postprovision_Propagates_Context_Cancellation to cover the ctx.Err() safety valve in skipPostprovisionK8sSetup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback — event guards, constants, test assertions - Route targetResource==nil through event-aware postprovisionK8sError so predeploy doesn't silently skip (thread #1, High) - Add preDeployEvent constant replacing raw "predeploy" strings for consistency with postProvisionEvent (thread #2) - Add console message assertions to graceful-skip tests verifying the warning was actually emitted (thread #3) - Add predeploy failure tests for credential and namespace error paths confirming errors propagate for non-postprovision events (thread #4) - Refactor createAksServiceTarget to delegate to createAksServiceTargetWithResourceManager, eliminating ~40 lines of duplication (thread #5) - Add structured telemetry span (AksPostprovisionSkipEvent) to the graceful-skip path for production monitoring (thread #6) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address re-review - telemetry span, DRY assertions, ErrorContains - Use span.End() with skip.reason attribute instead of EndWithStatus() to avoid marking intentional skips as errors in telemetry dashboards - Extract assertSkipWarningEmitted helper to deduplicate console assertion blocks across two test functions - Add ErrorContains assertion in Test_Predeploy_Fails_When_Credentials_Fail for consistency with sibling namespace test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: narrow postprovision graceful skip to not-found only Address @weikanglim's feedback: limit the graceful skip path to the explicit delayed-provisioning case (empty ResourceName) only. GetTargetResource, ensureClusterContext, and ensureNamespace errors now propagate during postprovision — real failures are no longer masked. Removed postprovisionK8sError helper. Fixed gofmt import ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6f0ed59 commit e5cd73b

3 files changed

Lines changed: 595 additions & 77 deletions

File tree

cli/azd/internal/tracing/events/events.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,10 @@ const (
4545
// and its outcome (passed, warnings accepted, aborted).
4646
PreflightValidationEvent = "validation.preflight"
4747
)
48+
49+
// AKS service target events.
50+
const (
51+
// AksPostprovisionSkipEvent tracks when the AKS postprovision hook
52+
// skips Kubernetes context setup because the cluster isn't available yet.
53+
AksPostprovisionSkipEvent = "aks.postprovision.skip"
54+
)

cli/azd/pkg/project/service_target_aks.go

Lines changed: 74 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"time"
1616

1717
"github.com/azure/azure-dev/cli/azd/internal/mapper"
18+
"github.com/azure/azure-dev/cli/azd/internal/tracing"
19+
"github.com/azure/azure-dev/cli/azd/internal/tracing/events"
1820
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
1921
"github.com/azure/azure-dev/cli/azd/pkg/async"
2022
"github.com/azure/azure-dev/cli/azd/pkg/azapi"
@@ -30,6 +32,7 @@ import (
3032
"github.com/azure/azure-dev/cli/azd/pkg/tools"
3133
"github.com/azure/azure-dev/cli/azd/pkg/tools/kubectl"
3234
"github.com/sethvargo/go-retry"
35+
"go.opentelemetry.io/otel/attribute"
3336
)
3437

3538
const (
@@ -135,15 +138,22 @@ func (t *aksTarget) RequiredExternalTools(ctx context.Context, serviceConfig *Se
135138
return allTools
136139
}
137140

141+
// postProvisionEvent is the event fired after provisioning completes.
142+
// Defined as a constant to avoid magic string repetition.
143+
const postProvisionEvent = ext.Event("post" + string(ProjectEventProvision))
144+
145+
// preDeployEvent is the service-level event fired before deployment.
146+
const preDeployEvent = ext.Event("pre" + string(ServiceEventDeploy))
147+
138148
// Initializes the AKS service target
139149
func (t *aksTarget) Initialize(ctx context.Context, serviceConfig *ServiceConfig) error {
140150
// Ensure that the k8s context has been configured by the time a deploy operation is performed.
141151
// We attach to "postprovision" so that any predeploy or postprovision hooks can take advantage of the configuration
142152
err := serviceConfig.Project.AddHandler(
143153
ctx,
144-
"postprovision",
154+
postProvisionEvent,
145155
func(ctx context.Context, args ProjectLifecycleEventArgs) error {
146-
return t.setK8sContext(ctx, serviceConfig, "postprovision")
156+
return t.setK8sContext(ctx, serviceConfig, postProvisionEvent)
147157
},
148158
)
149159

@@ -153,8 +163,8 @@ func (t *aksTarget) Initialize(ctx context.Context, serviceConfig *ServiceConfig
153163

154164
// Ensure that the k8s context has been configured by the time a deploy operation is performed.
155165
// We attach to "predeploy" so that any predeploy hooks can take advantage of the configuration
156-
err = serviceConfig.AddHandler(ctx, "predeploy", func(ctx context.Context, args ServiceLifecycleEventArgs) error {
157-
return t.setK8sContext(ctx, serviceConfig, "predeploy")
166+
err = serviceConfig.AddHandler(ctx, preDeployEvent, func(ctx context.Context, args ServiceLifecycleEventArgs) error {
167+
return t.setK8sContext(ctx, serviceConfig, preDeployEvent)
158168
})
159169

160170
if err != nil {
@@ -895,7 +905,11 @@ func (t *aksTarget) getK8sNamespace(serviceConfig *ServiceConfig) string {
895905
return namespace
896906
}
897907

898-
func (t *aksTarget) setK8sContext(ctx context.Context, serviceConfig *ServiceConfig, eventName ext.Event) error {
908+
func (t *aksTarget) setK8sContext(
909+
ctx context.Context,
910+
serviceConfig *ServiceConfig,
911+
eventName ext.Event,
912+
) error {
899913
t.kubectl.SetEnv(t.env.Dotenv())
900914
hasCustomKubeConfig := false
901915

@@ -906,13 +920,30 @@ func (t *aksTarget) setK8sContext(ctx context.Context, serviceConfig *ServiceCon
906920
hasCustomKubeConfig = true
907921
}
908922

909-
targetResource, err := t.resourceManager.GetTargetResource(ctx, t.env.GetSubscriptionId(), serviceConfig)
923+
targetResource, err := t.resourceManager.GetTargetResource(
924+
ctx, t.env.GetSubscriptionId(), serviceConfig)
910925
if err != nil {
911926
return err
912927
}
913928

929+
if targetResource == nil {
930+
return fmt.Errorf("AKS cluster target resource is nil")
931+
}
932+
933+
// When the AKS cluster hasn't been provisioned yet the target resource
934+
// will have an empty resource name (via SupportsDelayedProvisioning).
935+
// During postprovision this is expected in multi-phase provisioning
936+
// workflows — skip gracefully instead of failing. All other errors
937+
// (credentials, RBAC, namespace) are real failures that should surface
938+
// even during postprovision.
939+
if targetResource.ResourceName() == "" && eventName == postProvisionEvent {
940+
return t.skipPostprovisionK8sSetup(
941+
ctx, fmt.Errorf("AKS cluster resource not yet provisioned"))
942+
}
943+
914944
defaultNamespace := t.getK8sNamespace(serviceConfig)
915-
_, err = t.ensureClusterContext(ctx, serviceConfig, targetResource, defaultNamespace)
945+
_, err = t.ensureClusterContext(
946+
ctx, serviceConfig, targetResource, defaultNamespace)
916947
if err != nil {
917948
return err
918949
}
@@ -922,15 +953,47 @@ func (t *aksTarget) setK8sContext(ctx context.Context, serviceConfig *ServiceCon
922953
return err
923954
}
924955

925-
// Display message to the user when we detect they are using a non-default KUBECONFIG configuration
926-
// In standard AZD AKS deployment users should not typically need to set a custom KUBECONFIG
927-
if hasCustomKubeConfig && eventName == "predeploy" {
928-
t.console.Message(ctx, output.WithWarningFormat("Using KUBECONFIG @ %s\n", kubeConfigPath))
956+
// Display message to the user when we detect they are using a
957+
// non-default KUBECONFIG configuration. In standard AZD AKS deployment
958+
// users should not typically need to set a custom KUBECONFIG.
959+
if hasCustomKubeConfig && eventName == preDeployEvent {
960+
t.console.Message(
961+
ctx,
962+
output.WithWarningFormat(
963+
"Using KUBECONFIG @ %s\n", kubeConfigPath))
929964
}
930965

931966
return nil
932967
}
933968

969+
// skipPostprovisionK8sSetup logs a warning and returns nil so that
970+
// postprovision continues even when Kubernetes context setup fails.
971+
// The context will be configured later when a deployment is performed.
972+
// Context cancellation/timeouts are always propagated.
973+
func (t *aksTarget) skipPostprovisionK8sSetup(
974+
ctx context.Context,
975+
reason error,
976+
) error {
977+
// Propagate context cancellation — the user (or system) asked
978+
// to stop; swallowing that would be incorrect.
979+
if ctx.Err() != nil {
980+
return ctx.Err()
981+
}
982+
983+
_, span := tracing.Start(ctx, events.AksPostprovisionSkipEvent)
984+
span.SetAttributes(attribute.String("skip.reason", reason.Error()))
985+
span.End()
986+
987+
log.Printf(
988+
"skipping k8s context setup during postprovision: %v", reason)
989+
t.console.Message(ctx, output.WithWarningFormat(
990+
"AKS cluster not available yet, skipping Kubernetes "+
991+
"context setup. It will be configured when the "+
992+
"cluster is provisioned and a deployment is "+
993+
"performed.\n"))
994+
return nil
995+
}
996+
934997
// resolveClusterName attempts to resolve the cluster name from the following sources:
935998
// 1. The 'AZD_AKS_CLUSTER' environment variable
936999
// 2. The 'resourceName' property in the azure.yaml (Can use expandable string as well)

0 commit comments

Comments
 (0)