Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ func (api *HTTPAPI) HandleChangeCommitments(w http.ResponseWriter, r *http.Reque
return
}

if req.AZ == "" {
statusCode = http.StatusBadRequest
http.Error(w, "availability zone is required", statusCode)
api.recordMetrics(req, resp, statusCode, startTime)
return
}

if err := api.processCommitmentChanges(ctx, w, logger, req, &resp); err != nil {
if strings.Contains(err.Error(), "version mismatch") {
statusCode = http.StatusConflict
Expand Down Expand Up @@ -255,26 +262,31 @@ ProcessLoop:
break ProcessLoop
}

cr := &v1alpha1.CommittedResource{}
cr.Name = crName
if _, err := controllerutil.CreateOrUpdate(ctx, api.client, cr, func() error {
if cr.Spec.AvailabilityZone != "" && cr.Spec.AvailabilityZone != stateDesired.AvailabilityZone {
return fmt.Errorf("cannot change availability zone of commitment %s: current=%q requested=%q",
commitment.UUID, cr.Spec.AvailabilityZone, stateDesired.AvailabilityZone)
}
applyCRSpec(cr, stateDesired, allowRejection)
if cr.Annotations == nil {
cr.Annotations = make(map[string]string)
// RetryOnConflict handles the race where the CommittedResource controller reconciles
// the CRD (bumping resourceVersion) between the Get and Update inside CreateOrUpdate.
var crGeneration int64
if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
cr := &v1alpha1.CommittedResource{}
cr.Name = crName
if _, err := controllerutil.CreateOrUpdate(ctx, api.client, cr, func() error {
applyCRSpec(cr, stateDesired, allowRejection)
if cr.Annotations == nil {
cr.Annotations = make(map[string]string)
}
cr.Annotations[v1alpha1.AnnotationCreatorRequestID] = reservations.GlobalRequestIDFromContext(ctx)
return nil
}); err != nil {
return err
}
cr.Annotations[v1alpha1.AnnotationCreatorRequestID] = reservations.GlobalRequestIDFromContext(ctx)
crGeneration = cr.Generation
return nil
}); err != nil {
failedReason = fmt.Sprintf("commitment %s: failed to write CommittedResource CRD: %v", commitment.UUID, err)
rollback = true
break ProcessLoop
}

toWatch = append(toWatch, crWatch{name: crName, generation: cr.Generation})
toWatch = append(toWatch, crWatch{name: crName, generation: crGeneration})
snapshots = append(snapshots, snap)
logger.V(1).Info("upserted CommittedResource CRD", "name", crName)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,13 @@ func TestHandleChangeCommitments(t *testing.T) {
ExpectedAPIResponse: newAPIResponse("uuid-b: not sufficient capacity"),
ExpectedDeletedCRs: []string{"commitment-uuid-a", "commitment-uuid-b"},
},
// --- AZ immutability ---
// --- AZ validation ---
{
// AZ is immutable once set on a CommittedResource. Attempting to change it via
// change-commitments must be rejected immediately, before any polling or controller
// interaction, and the CR must remain at its original spec.
Name: "AZ change on existing CR: must be rejected",
Name: "Empty AZ: rejected with 400",
Flavors: []*TestFlavor{m1Small},
ExistingCRs: []*TestCR{
{CommitmentUUID: "uuid-az-stale", State: v1alpha1.CommitmentStatusConfirmed,
AmountMiB: 1024, ProjectID: "project-A", AZ: "az-old", ReadyCondition: true},
},
CommitmentRequest: newCommitmentRequest("az-new", false, 1234,
createCommitment("hw_version_hana_1_ram", "project-A", "uuid-az-stale", "confirmed", 2)),
ExpectedAPIResponse: newAPIResponse("cannot change availability zone"),
// CR spec must not have changed.
ExpectedCRSpecs: map[string]int64{"commitment-uuid-az-stale": 1024 * 1024 * 1024},
CommitmentRequest: newCommitmentRequest("", false, 1234,
createCommitment("hw_version_hana_1_ram", "project-A", "uuid-noaz", "confirmed", 2)),
ExpectedAPIResponse: APIResponseExpectation{StatusCode: http.StatusBadRequest},
},
// --- Timeout ---
{
Expand Down
Loading