fix: skip experiment weight destinations with empty ServiceName

[tanmay-agrawal-india]

Fixes #4890

During experiment teardown the experiment's template statuses can momentarily carry an empty serviceName while the template weight from the step spec still applies. calculateWeightDestinationsFromExperiment copied the empty ServiceName into a WeightDestination, which the Istio reconciler renders as a VirtualService route destination with an empty host carrying the arm's weight. Envoy cannot route such destinations, so that traffic share is black-holed (503s), and the entries are never cleaned up — recovery requires manually restoring the VirtualService. I hit this in a production environment I operate, during normal experiment teardown.

This change skips template statuses with an empty ServiceName (logging a warning) so a route destination without a host can never be emitted, and adds a regression test simulating the teardown race.

The controller's own event stream shows the poisoned state this prevents:

TrafficWeightUpdated: Traffic weight updated additional: [{1  } {1  }]

Alternatives considered

• Gate on IsTerminating instead of checking the ServiceName: terminating is only one of several ways the status can carry a blank ServiceName (Service-creation failure at experiment start, template Failed/Error, external status writes — the experiments CRD has no status subresource). The empty-name check enforces the invariant itself — never emit a destination without a host — rather than one of its causes.
• Order teardown so the VirtualService is cleaned before arm Services are deleted: cross-controller ordering fights the level-triggered model and can wedge teardown when the rollout controller is slow or unavailable. Tolerating a mid-transition snapshot is the idiomatic contract.
• Treat the condition as experiment termination and advance/abort the rollout: a blank ServiceName is observed-state lag, not a lifecycle signal — it also occurs during healthy teardown seconds before Phase leaves Running; acting on it would abort healthy releases. Lifecycle decisions stay with experiment phase + analysis results.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number.
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
28.2% Duplication on New Code

See analysis details on SonarQube Cloud

[tanmay-agrawal-india]

Adding the exact upstream code path that opens the window, for reviewers — both halves are in-tree:

Producer side (experiments controller) — experiments/experiment.go:

• createTemplateService records the Service in status only after creation succeeds: templateStatus.ServiceName = newService.Name (the spec's service.name is optional — when omitted the name is generated, so status is the only authoritative record of what exists).
• When a template's desired replica count reaches 0 — the template completed, or the experiment is terminating (duration elapsed, abort, Spec.Terminate, or a failed/errored template) — and its pods have drained (rs.Status.AvailableReplicas == 0), deleteTemplateService deletes the Service and blanks the status fields in the same reconcile:

ec.templateServices[templateName] = nil
templateStatus.ServiceName = ""
templateStatus.PodTemplateHash = ""

Because the AvailableReplicas == 0 gate depends on pod drain timing, the exact moment this lands is unpredictable — and it can land while Status.Phase is still Running (Spec.Terminate = true does not immediately move the phase).

Consumer side (rollout controller) — rollout/trafficrouting.go: calculateWeightDestinationsFromExperiment gates only on c.currentEx.Status.Phase == v1alpha1.AnalysisPhaseRunning and takes the arm weight from the step spec. In the window above it reads a blank ServiceName with a live weight and emits the empty WeightDestination.

Since WeightDestination is consumed by every traffic router (istio, smi, apisix, and third-party RPC plugins), guarding the single producer covers them all — which is why the fix is here rather than in the Istio reconciler.

Degradation with the guard is the safe direction: for the reconciles inside the window the arm simply receives no traffic share (identical to the post-teardown steady state), and the next reconcile with a repopulated status converges normally — no ordering between the two controllers is introduced.