diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index f14b252085..a7bb2fc8ee 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -182,7 +182,7 @@ func New( // controller that loads and applies content to the cluster. It returns an error if the payload appears to // be in error rather than continuing. func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestConfig *rest.Config) error { - update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.releaseImage) + update, err := payload.Load(optr.defaultPayloadDir(), optr.releaseImage) if err != nil { return fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err) } diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go index 3cc8d3c4ef..7acadf37f8 100644 --- a/pkg/cvo/cvo_scenarios_test.go +++ b/pkg/cvo/cvo_scenarios_test.go @@ -357,7 +357,7 @@ func TestCVO_StartupAndSyncUnverifiedPayload(t *testing.T) { defer shutdownFn() // make the image report unverified - payloadErr := &payload.UpdateError{ + payloadErr := &payload.Error{ Reason: "ImageVerificationFailed", Message: fmt.Sprintf("The update cannot be verified: some random error"), Nested: fmt.Errorf("some random error"), @@ -661,7 +661,7 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { defer shutdownFn() // make the image report unverified - payloadErr := &payload.UpdateError{ + payloadErr := &payload.Error{ Reason: "ImageVerificationFailed", Message: fmt.Sprintf("The update cannot be verified: some random error"), Nested: fmt.Errorf("some random error"), @@ -887,7 +887,7 @@ func TestCVO_UpgradeVerifiedPayload(t *testing.T) { defer shutdownFn() // make the image report unverified - payloadErr := &payload.UpdateError{ + payloadErr := &payload.Error{ Reason: "ImageVerificationFailed", Message: fmt.Sprintf("The update cannot be verified: some random error"), Nested: fmt.Errorf("some random error"), @@ -1385,10 +1385,10 @@ func TestCVO_ErrorDuringReconcile(t *testing.T) { Step: "ApplyResources", Fraction: float32(2) / 3, VersionHash: "6GC9TkkG9PA=", - Failure: &payload.UpdateError{ + Failure: &payload.Error{ Nested: fmt.Errorf("unable to proceed"), - Reason: "UpdatePayloadFailed", - Message: "Could not update test \"file-yml\" (3 of 3)", + Reason: "ApplyManifestError", + Message: "Could not apply test \"file-yml\" (3 of 3): unable to proceed", Task: &payload.Task{Index: 3, Total: 3, Manifest: &worker.payload.Manifests[2]}, }, Actual: configv1.Update{Version: "1.0.0-abc", Image: "image/image:1"}, @@ -1423,8 +1423,8 @@ func TestCVO_ErrorDuringReconcile(t *testing.T) { }, Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue, Message: "Done applying 1.0.0-abc"}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadFailed", Message: "Could not update test \"file-yml\" (3 of 3)"}, - {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "UpdatePayloadFailed", Message: "Error while reconciling 1.0.0-abc: the update could not be applied"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "ApplyManifestError", Message: "Could not apply test \"file-yml\" (3 of 3): unable to proceed"}, + {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "ApplyManifestError", Message: "Error while reconciling 1.0.0-abc: Could not apply test \"file-yml\" (3 of 3): unable to proceed"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, }, @@ -1440,12 +1440,12 @@ func TestCVO_ParallelError(t *testing.T) { defer shutdownFn() worker := o.configSync.(*SyncWorker) b := &errorResourceBuilder{errors: map[string]error{ - "0000_10_a_file.yaml": &payload.UpdateError{ + "0000_10_a_file.yaml": &payload.Error{ Reason: "ClusterOperatorNotAvailable", Name: "operator-1", }, "0000_20_a_file.yaml": nil, - "0000_20_b_file.yaml": &payload.UpdateError{ + "0000_20_b_file.yaml": &payload.Error{ Reason: "ClusterOperatorNotAvailable", Name: "operator-2", }, @@ -1539,7 +1539,7 @@ func TestCVO_ParallelError(t *testing.T) { continue } err := status.Failure - uErr, ok := err.(*payload.UpdateError) + uErr, ok := err.(*payload.Error) if !ok || uErr.Reason != "ClusterOperatorsNotAvailable" || uErr.Message != "Some cluster operators are still updating: operator-1, operator-2" { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index 4a296c326c..a34c2a0765 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -227,8 +227,8 @@ func TestOperator_sync(t *testing.T) { Step: "Moving", Reconciling: false, Actual: configv1.Update{Version: "0.0.1-abc", Image: "image/image:v4.0.1"}, - Failure: &payload.UpdateError{ - Reason: "UpdatePayloadIntegrity", + Failure: &payload.Error{ + Reason: "LoadManifestsError", Message: "unable to apply object", }, }, @@ -253,7 +253,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "Working towards 4.0.1"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, @@ -284,8 +284,8 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, - {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "Unable to apply 0.0.1-abc: the contents of the update are invalid"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, + {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "Unable to apply 0.0.1-abc: failed to load manifests from the release image"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, }, @@ -304,8 +304,8 @@ func TestOperator_sync(t *testing.T) { Step: "Moving", Reconciling: true, Actual: configv1.Update{Version: "0.0.1-abc", Image: "image/image:v4.0.1"}, - Failure: &payload.UpdateError{ - Reason: "UpdatePayloadIntegrity", + Failure: &payload.Error{ + Reason: "LoadManifestsError", Message: "unable to apply object", }, }, @@ -326,7 +326,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "Working towards 4.0.1"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, @@ -357,8 +357,8 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, - {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "UpdatePayloadIntegrity", Message: "Error while reconciling 0.0.1-abc: the contents of the update are invalid"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, + {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "LoadManifestsError", Message: "Error while reconciling 0.0.1-abc: failed to load manifests from the release image"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, }, @@ -378,8 +378,8 @@ func TestOperator_sync(t *testing.T) { Reconciling: true, Completed: 2, Actual: configv1.Update{Version: "0.0.1-abc", Image: "image/image:v4.0.1"}, - Failure: &payload.UpdateError{ - Reason: "UpdatePayloadIntegrity", + Failure: &payload.Error{ + Reason: "LoadManifestsError", Message: "unable to apply object", }, }, @@ -400,7 +400,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "Working towards 4.0.1"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, @@ -431,8 +431,8 @@ func TestOperator_sync(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue, Message: "Done applying 0.0.1-abc"}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, - {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "UpdatePayloadIntegrity", Message: "Error while reconciling 0.0.1-abc: the contents of the update are invalid"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, + {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, Reason: "LoadManifestsError", Message: "Error while reconciling 0.0.1-abc: failed to load manifests from the release image"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, }, diff --git a/pkg/cvo/internal/operatorstatus.go b/pkg/cvo/internal/operatorstatus.go index cda58cd74b..62554f989d 100644 --- a/pkg/cvo/internal/operatorstatus.go +++ b/pkg/cvo/internal/operatorstatus.go @@ -105,7 +105,7 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration, err := wait.PollImmediateUntil(interval, func() (bool, error) { actual, err := client.Get(expected.Name) if err != nil { - lastErr = &payload.UpdateError{ + lastErr = &payload.Error{ Nested: err, Reason: "ClusterOperatorNotAvailable", Message: fmt.Sprintf("Cluster operator %s has not yet reported success", expected.Name), @@ -137,7 +137,7 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration, sort.Strings(keys) message := fmt.Sprintf("Cluster operator %s is still updating", actual.Name) - lastErr = &payload.UpdateError{ + lastErr = &payload.Error{ Nested: errors.New(lowerFirst(message)), Reason: "ClusterOperatorNotAvailable", Message: message, @@ -197,7 +197,7 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration, if len(condition.Message) > 0 { message = fmt.Sprintf("Cluster operator %s is reporting a failure: %s", actual.Name, condition.Message) } - lastErr = &payload.UpdateError{ + lastErr = &payload.Error{ Nested: errors.New(lowerFirst(message)), Reason: "ClusterOperatorDegraded", Message: message, @@ -206,7 +206,7 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration, return false, nil } - lastErr = &payload.UpdateError{ + lastErr = &payload.Error{ Nested: fmt.Errorf("cluster operator %s is not done; it is available=%v, progressing=%v, degraded=%v", actual.Name, available, progressing, degraded, ), diff --git a/pkg/cvo/internal/operatorstatus_test.go b/pkg/cvo/internal/operatorstatus_test.go index 4ccbc90aca..14d045209e 100644 --- a/pkg/cvo/internal/operatorstatus_test.go +++ b/pkg/cvo/internal/operatorstatus_test.go @@ -42,7 +42,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", @@ -61,7 +61,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -82,7 +82,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -108,7 +108,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -136,7 +136,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -164,7 +164,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -196,7 +196,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -228,7 +228,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is still updating"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co is still updating", @@ -256,7 +256,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", @@ -285,7 +285,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", @@ -314,7 +314,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"), Reason: "ClusterOperatorDegraded", Message: "Cluster operator test-co is reporting a failure: random error", @@ -343,7 +343,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", @@ -372,7 +372,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"), Reason: "ClusterOperatorDegraded", Message: "Cluster operator test-co is reporting a failure: random error", @@ -401,7 +401,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"), Reason: "ClusterOperatorDegraded", Message: "Cluster operator test-co is reporting a failure: random error", @@ -430,7 +430,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) { }}, }, }, - expErr: &payload.UpdateError{ + expErr: &payload.Error{ Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"), Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", diff --git a/pkg/cvo/status.go b/pkg/cvo/status.go index 25f2639bae..b64e9062a5 100644 --- a/pkg/cvo/status.go +++ b/pkg/cvo/status.go @@ -221,9 +221,9 @@ func (optr *Operator) syncStatus(original, config *configv1.ClusterVersion, stat if err := status.Failure; err != nil && !skipFailure { var reason string msg := "an error occurred" - if uErr, ok := err.(*payload.UpdateError); ok { + if uErr, ok := err.(*payload.Error); ok { reason = uErr.Reason - msg = payload.SummaryForReason(reason, uErr.Name) + msg = uErr.Summary() } // set the failing condition @@ -330,7 +330,7 @@ func convertErrorToProgressing(history []configv1.UpdateHistory, now time.Time, if now.Sub(status.LastProgress) > 10*time.Minute || now.Sub(history[0].StartedTime.Time) > time.Hour { return "", "", false } - uErr, ok := status.Failure.(*payload.UpdateError) + uErr, ok := status.Failure.(*payload.Error) if !ok { return "", "", false } diff --git a/pkg/cvo/status_test.go b/pkg/cvo/status_test.go index d8c2e75c2e..62392c94bd 100644 --- a/pkg/cvo/status_test.go +++ b/pkg/cvo/status_test.go @@ -177,7 +177,7 @@ func TestOperator_syncFailingStatus(t *testing.T) { VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ {Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, - {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "UpdatePayloadIntegrity", Message: "unable to apply object"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "LoadManifestsError", Message: "unable to apply object"}, {Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "Working towards 4.0.1"}, {Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse}, }, diff --git a/pkg/cvo/sync_test.go b/pkg/cvo/sync_test.go index 6121d14898..1153b9c4ba 100644 --- a/pkg/cvo/sync_test.go +++ b/pkg/cvo/sync_test.go @@ -116,7 +116,7 @@ func Test_SyncWorker_apply(t *testing.T) { manifests = append(manifests, m) } - up := &payload.Update{ReleaseImage: "test", ReleaseVersion: "v0.0.0", Manifests: manifests} + up := &payload.Payload{ReleaseImage: "test", ReleaseVersion: "v0.0.0", Manifests: manifests} r := &recorder{} testMapper := resourcebuilder.NewResourceMapper() testMapper.RegisterGVK(schema.GroupVersionKind{"test.cvo.io", "v1", "TestA"}, newTestBuilder(r, test.reactors)) @@ -308,7 +308,7 @@ func Test_SyncWorker_apply_generic(t *testing.T) { dynamicScheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestB"}, &unstructured.Unstructured{}) dynamicClient := dynamicfake.NewSimpleDynamicClient(dynamicScheme) - up := &payload.Update{ReleaseImage: "test", ReleaseVersion: "v0.0.0", Manifests: manifests} + up := &payload.Payload{ReleaseImage: "test", ReleaseVersion: "v0.0.0", Manifests: manifests} worker := &SyncWorker{} worker.backoff.Steps = 1 worker.builder = &testResourceBuilder{ diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go index d0fe115e50..1f8a507698 100644 --- a/pkg/cvo/sync_worker.go +++ b/pkg/cvo/sync_worker.go @@ -145,7 +145,7 @@ type SyncWorker struct { status SyncWorkerStatus // updated by the run method only - payload *payload.Update + payload *payload.Payload } // NewSyncWorker initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder @@ -477,7 +477,7 @@ func (w *SyncWorker) syncOnce(ctx context.Context, work *SyncWork, maxWorkers in return err } - payloadUpdate, err := payload.LoadUpdate(info.Directory, update.Image) + payloadUpdate, err := payload.Load(info.Directory, update.Image) if err != nil { reporter.Report(SyncWorkerStatus{ Generation: work.Generation, @@ -503,7 +503,7 @@ func (w *SyncWorker) syncOnce(ctx context.Context, work *SyncWork, maxWorkers in // apply updates the server with the contents of the provided image or returns an error. // Cancelling the context will abort the execution of the sync. Will be executed in parallel if // maxWorkers is set greater than 1. -func (w *SyncWorker) apply(ctx context.Context, payloadUpdate *payload.Update, work *SyncWork, maxWorkers int, reporter StatusReporter) error { +func (w *SyncWorker) apply(ctx context.Context, payloadUpdate *payload.Payload, work *SyncWork, maxWorkers int, reporter StatusReporter) error { update := configv1.Update{ Version: payloadUpdate.ReleaseVersion, Image: payloadUpdate.ReleaseImage, @@ -704,7 +704,7 @@ func isImageVerificationError(err error) bool { if err == nil { return false } - updateErr, ok := err.(*payload.UpdateError) + updateErr, ok := err.(*payload.Error) if !ok { return false } @@ -735,7 +735,7 @@ func summarizeTaskGraphErrors(errs []error) error { if klog.V(4) { klog.Infof("Summarizing %d errors", len(errs)) for _, err := range errs { - if uErr, ok := err.(*payload.UpdateError); ok { + if uErr, ok := err.(*payload.Error); ok { if uErr.Task != nil { klog.Infof("Update error %d/%d: %s %s (%T: %v)", uErr.Task.Index, uErr.Task.Total, uErr.Reason, uErr.Message, uErr.Nested, uErr.Nested) } else { @@ -762,7 +762,7 @@ func summarizeTaskGraphErrors(errs []error) error { func newClusterOperatorsNotAvailable(errs []error) error { names := make([]string, 0, len(errs)) for _, err := range errs { - uErr, ok := err.(*payload.UpdateError) + uErr, ok := err.(*payload.Error) if !ok || uErr.Reason != "ClusterOperatorNotAvailable" { return nil } @@ -780,7 +780,7 @@ func newClusterOperatorsNotAvailable(errs []error) error { } sort.Strings(names) name := strings.Join(names, ", ") - return &payload.UpdateError{ + return &payload.Error{ Nested: errors.NewAggregate(errs), Reason: "ClusterOperatorsNotAvailable", Message: fmt.Sprintf("Some cluster operators are still updating: %s", name), @@ -826,7 +826,7 @@ func newMultipleError(errs []error) error { if len(messages) == 0 { return errs[0] } - return &payload.UpdateError{ + return &payload.Error{ Nested: errors.NewAggregate(errs), Reason: "MultipleErrors", Message: fmt.Sprintf("Multiple errors are preventing progress:\n* %s", strings.Join(messages, "\n* ")), diff --git a/pkg/cvo/updatepayload.go b/pkg/cvo/updatepayload.go index abf490e5fe..7d74a6663c 100644 --- a/pkg/cvo/updatepayload.go +++ b/pkg/cvo/updatepayload.go @@ -85,7 +85,7 @@ func (r *payloadRetriever) RetrievePayload(ctx context.Context, update configv1. releaseDigest = update.Image[index+1:] } if err := r.verifier.Verify(ctx, releaseDigest); err != nil { - vErr := &payload.UpdateError{ + vErr := &payload.Error{ Reason: "ImageVerificationFailed", Message: fmt.Sprintf("The update cannot be verified: %v", err), Nested: err, @@ -103,7 +103,7 @@ func (r *payloadRetriever) RetrievePayload(ctx context.Context, update configv1. var err error info.Directory, err = r.targetUpdatePayloadDir(ctx, update) if err != nil { - return PayloadInfo{}, &payload.UpdateError{ + return PayloadInfo{}, &payload.Error{ Reason: "UpdatePayloadRetrievalFailed", Message: fmt.Sprintf("Unable to download and prepare the update: %v", err), } diff --git a/pkg/payload/error.go b/pkg/payload/error.go new file mode 100644 index 0000000000..22c8e84ba4 --- /dev/null +++ b/pkg/payload/error.go @@ -0,0 +1,91 @@ +package payload + +import ( + "fmt" + "strings" + + "github.com/pkg/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" +) + +// Error is a wrapper for errors that occur during a payload sync. +type Error struct { + Nested error + Reason string + Message string + Name string + + Task *Task +} + +func (e *Error) Error() string { + return e.Message +} + +func (e *Error) Cause() error { + return e.Nested +} + +func (e *Error) Summary() string { + switch e.Reason { + + case "ApplyManifestError": + return e.Message + + case "LoadManifestsError": + return "failed to load manifests from the release image" + + case "ImageVerificationFailed": + return "the image may not be safe to use" + + case "ClusterOperatorDegraded": + if len(e.Name) > 0 { + return fmt.Sprintf("the cluster operator %s is degraded", e.Name) + } + return "a cluster operator is degraded" + case "ClusterOperatorNotAvailable": + if len(e.Name) > 0 { + return fmt.Sprintf("the cluster operator %s has not yet successfully rolled out", e.Name) + } + return "a cluster operator has not yet rolled out" + case "ClusterOperatorsNotAvailable": + return "some cluster operators have not yet rolled out" + } + + if strings.HasPrefix(e.Reason, "UpdatePayload") { + return "the update could not be applied" + } + return "an unknown error has occurred" +} + +// messageForError provides a succint explanation of a known error type +// for use in a human readable message. Since all objects in the image +// should be successfully applied, messages should direct the reader +// (likely a cluster administrator) to a possible cause in their own +// config. +func messageForError(err error) string { + err = errors.Cause(err) + switch { + case apierrors.IsNotFound(err), apierrors.IsAlreadyExists(err): + return "resource may have been deleted" + case apierrors.IsConflict(err): + return "someone else is updating this resource" + case apierrors.IsTimeout(err), apierrors.IsServiceUnavailable(err), apierrors.IsUnexpectedServerError(err): + return "the control plane is down or not responding" + case apierrors.IsInternalError(err): + return "the control plane is reporting an internal error" + case apierrors.IsInvalid(err): + return "the object is invalid, possibly due to local cluster configuration" + case apierrors.IsUnauthorized(err): + return "could not authenticate to the control plane" + case apierrors.IsForbidden(err): + return "the control plane has forbidden updates to this resource" + case apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err): + return "the control plane is overloaded and is not accepting updates" + case meta.IsNoMatchError(err): + return "the control plane does not recognize this resource, check extension API servers" + default: + return err.Error() + } +} diff --git a/pkg/payload/image.go b/pkg/payload/image.go index f282a9d223..0430fed70b 100644 --- a/pkg/payload/image.go +++ b/pkg/payload/image.go @@ -6,10 +6,10 @@ import ( "github.com/pkg/errors" ) -// ImageForShortName returns the image using the updatepayload embedded in +// ImageForShortName returns the image using the payload embedded in // the Operator. func ImageForShortName(name string) (string, error) { - up, err := LoadUpdate(DefaultPayloadDir, "") + up, err := Load(DefaultPayloadDir, "") if err != nil { return "", errors.Wrapf(err, "error loading release manifests from %q", DefaultPayloadDir) } @@ -23,5 +23,5 @@ func ImageForShortName(name string) (string, error) { } } - return "", fmt.Errorf("error: Unknown name requested, could not find %s in UpdatePayload", name) + return "", fmt.Errorf("error: Unknown name requested, could not find %s in payload", name) } diff --git a/pkg/payload/payload.go b/pkg/payload/payload.go index 2e978526bf..d2f97fdff7 100644 --- a/pkg/payload/payload.go +++ b/pkg/payload/payload.go @@ -1,3 +1,4 @@ +// Package payload provides tools for interacting with the content of release images. package payload import ( @@ -87,7 +88,7 @@ const ( imageReferencesFile = "image-references" ) -type Update struct { +type Payload struct { ReleaseImage string ReleaseVersion string // XXX: cincinatti.json struct @@ -102,8 +103,9 @@ type Update struct { Manifests []lib.Manifest } -func LoadUpdate(dir, releaseImage string) (*Update, error) { - payload, tasks, err := loadUpdatePayloadMetadata(dir, releaseImage) +// Load loads a Payload from the given release image. +func Load(dir, releaseImage string) (*Payload, error) { + payload, tasks, err := loadPayloadMetadata(dir, releaseImage) if err != nil { return nil, err } @@ -158,8 +160,8 @@ func LoadUpdate(dir, releaseImage string) (*Update, error) { agg := utilerrors.NewAggregate(errs) if agg != nil { - return nil, &UpdateError{ - Reason: "UpdatePayloadIntegrity", + return nil, &Error{ + Reason: "LoadManifestsError", Message: fmt.Sprintf("Error loading manifests from %s: %v", dir, agg.Error()), } } @@ -206,8 +208,8 @@ type payloadTasks struct { skipFiles sets.String } -func loadUpdatePayloadMetadata(dir, releaseImage string) (*Update, []payloadTasks, error) { - klog.V(4).Infof("Loading updatepayload from %q", dir) +func loadPayloadMetadata(dir, releaseImage string) (*Payload, []payloadTasks, error) { + klog.V(4).Infof("Loading payload from %q", dir) if err := ValidateDirectory(dir); err != nil { return nil, nil, err } @@ -240,5 +242,5 @@ func loadUpdatePayloadMetadata(dir, releaseImage string) (*Update, []payloadTask preprocess: nil, skipFiles: sets.NewString(cjf, irf), }} - return &Update{ImageRef: imageRef, ReleaseImage: releaseImage, ReleaseVersion: imageRef.Name}, tasks, nil + return &Payload{ImageRef: imageRef, ReleaseImage: releaseImage, ReleaseVersion: imageRef.Name}, tasks, nil } diff --git a/pkg/payload/payload_test.go b/pkg/payload/payload_test.go index 376d3f1586..7546959b07 100644 --- a/pkg/payload/payload_test.go +++ b/pkg/payload/payload_test.go @@ -16,7 +16,7 @@ import ( "github.com/openshift/cluster-version-operator/lib" ) -func Test_loadUpdatePayload(t *testing.T) { +func Test_loadPayload(t *testing.T) { type args struct { dir string releaseImage string @@ -24,7 +24,7 @@ func Test_loadUpdatePayload(t *testing.T) { tests := []struct { name string args args - want *Update + want *Payload wantErr bool }{ { @@ -33,7 +33,7 @@ func Test_loadUpdatePayload(t *testing.T) { dir: filepath.Join("..", "cvo", "testdata", "payloadtest"), releaseImage: "image:1", }, - want: &Update{ + want: &Payload{ ReleaseImage: "image:1", ReleaseVersion: "1.0.0-abc", ImageRef: &imagev1.ImageStream{ @@ -104,13 +104,13 @@ func Test_loadUpdatePayload(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := LoadUpdate(tt.args.dir, tt.args.releaseImage) + got, err := Load(tt.args.dir, tt.args.releaseImage) if (err != nil) != tt.wantErr { - t.Errorf("loadUpdatePayload() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("loadPayload() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("loadUpdatePayload() = %s", diff.ObjectReflectDiff(tt.want, got)) + t.Errorf("loadPayload() = %s", diff.ObjectReflectDiff(tt.want, got)) } }) } diff --git a/pkg/payload/task.go b/pkg/payload/task.go index 99da959453..f5f7d08ba8 100644 --- a/pkg/payload/task.go +++ b/pkg/payload/task.go @@ -9,8 +9,6 @@ import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -89,121 +87,16 @@ func (st *Task) Run(ctx context.Context, version string, builder ResourceBuilder case <-time.After(d): continue case <-ctx.Done(): - if uerr, ok := lastErr.(*UpdateError); ok { + if uerr, ok := lastErr.(*Error); ok { uerr.Task = st.Copy() return uerr } - reason, cause := reasonForPayloadSyncError(lastErr) - if len(cause) > 0 { - cause = ": " + cause - } - return &UpdateError{ + return &Error{ Nested: lastErr, - Reason: reason, - Message: fmt.Sprintf("Could not update %s%s", st, cause), - + Reason: "ApplyManifestError", + Message: fmt.Sprintf("Could not apply %s: %s", st, messageForError(lastErr)), Task: st.Copy(), } } } } - -// UpdateError is a wrapper for errors that occur during a payload sync. -type UpdateError struct { - Nested error - Reason string - Message string - Name string - - Task *Task -} - -func (e *UpdateError) Error() string { - return e.Message -} - -func (e *UpdateError) Cause() error { - return e.Nested -} - -// reasonForUpdateError provides a succint explanation of a known error type for use in a human readable -// message during update. Since all objects in the image should be successfully applied, messages -// should direct the reader (likely a cluster administrator) to a possible cause in their own config. -func reasonForPayloadSyncError(err error) (string, string) { - err = errors.Cause(err) - switch { - case apierrors.IsNotFound(err), apierrors.IsAlreadyExists(err): - return "UpdatePayloadResourceNotFound", "resource may have been deleted" - case apierrors.IsConflict(err): - return "UpdatePayloadResourceConflict", "someone else is updating this resource" - case apierrors.IsTimeout(err), apierrors.IsServiceUnavailable(err), apierrors.IsUnexpectedServerError(err): - return "UpdatePayloadClusterDown", "the server is down or not responding" - case apierrors.IsInternalError(err): - return "UpdatePayloadClusterError", "the server is reporting an internal error" - case apierrors.IsInvalid(err): - return "UpdatePayloadResourceInvalid", "the object is invalid, possibly due to local cluster configuration" - case apierrors.IsUnauthorized(err): - return "UpdatePayloadClusterUnauthorized", "could not authenticate to the server" - case apierrors.IsForbidden(err): - return "UpdatePayloadResourceForbidden", "the server has forbidden updates to this resource" - case apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err): - return "UpdatePayloadClusterOverloaded", "the server is overloaded and is not accepting updates" - case meta.IsNoMatchError(err): - return "UpdatePayloadResourceTypeMissing", "the server does not recognize this resource, check extension API servers" - default: - return "UpdatePayloadFailed", "" - } -} - -func SummaryForReason(reason, name string) string { - switch reason { - - // likely temporary errors - case "UpdatePayloadResourceNotFound", "UpdatePayloadResourceConflict": - return "some resources could not be updated" - case "UpdatePayloadClusterDown": - return "the control plane is down or not responding" - case "UpdatePayloadClusterError": - return "the control plane is reporting an internal error" - case "UpdatePayloadClusterOverloaded": - return "the control plane is overloaded and is not accepting updates" - case "UpdatePayloadClusterUnauthorized": - return "could not authenticate to the server" - case "UpdatePayloadRetrievalFailed": - return "could not download the update" - - // likely a policy or other configuration error due to end user action - case "UpdatePayloadResourceForbidden": - return "the server is rejecting updates" - - // the image may not be correct, or the cluster may be in an unexpected - // state - case "UpdatePayloadResourceTypeMissing": - return "a required extension is not available to update" - case "UpdatePayloadResourceInvalid": - return "some cluster configuration is invalid" - case "UpdatePayloadIntegrity": - return "the contents of the update are invalid" - - case "ImageVerificationFailed": - return "the image may not be safe to use" - - case "ClusterOperatorDegraded": - if len(name) > 0 { - return fmt.Sprintf("the cluster operator %s is degraded", name) - } - return "a cluster operator is degraded" - case "ClusterOperatorNotAvailable": - if len(name) > 0 { - return fmt.Sprintf("the cluster operator %s has not yet successfully rolled out", name) - } - return "a cluster operator has not yet rolled out" - case "ClusterOperatorsNotAvailable": - return "some cluster operators have not yet rolled out" - } - - if strings.HasPrefix(reason, "UpdatePayload") { - return "the update could not be applied" - } - return "an unknown error has occurred" -} diff --git a/pkg/payload/task_graph_test.go b/pkg/payload/task_graph_test.go index f5fc8afbad..e2496ee56e 100644 --- a/pkg/payload/task_graph_test.go +++ b/pkg/payload/task_graph_test.go @@ -375,7 +375,7 @@ func Test_TaskGraph_real(t *testing.T) { if len(path) == 0 { t.Skip("TEST_GRAPH_PATH unset") } - p, err := LoadUpdate(path, "arbitrary/image:1") + p, err := Load(path, "arbitrary/image:1") if err != nil { t.Fatal(err) } diff --git a/pkg/verify/verify.go b/pkg/verify/verify.go index bd6f287b0c..458087927f 100644 --- a/pkg/verify/verify.go +++ b/pkg/verify/verify.go @@ -74,7 +74,7 @@ var Reject Interface = rejectVerifier{} // if each provided public key has signed the release image digest. The signature may be in any // store and the lookup order is internally defined. // -func LoadFromPayload(update *payload.Update) (Interface, error) { +func LoadFromPayload(update *payload.Payload) (Interface, error) { configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap") for _, manifest := range update.Manifests { if manifest.GVK != configMapGVK { diff --git a/pkg/verify/verify_test.go b/pkg/verify/verify_test.go index 32ee408a72..293bdb981b 100644 --- a/pkg/verify/verify_test.go +++ b/pkg/verify/verify_test.go @@ -171,18 +171,18 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) { tests := []struct { name string - update *payload.Update + update *payload.Payload want bool wantErr bool wantVerifiers int }{ { name: "is a no-op when no objects are found", - update: &payload.Update{}, + update: &payload.Payload{}, }, { name: "requires data", - update: &payload.Update{ + update: &payload.Payload{ Manifests: []lib.Manifest{ { GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, @@ -204,7 +204,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) { }, { name: "requires stores", - update: &payload.Update{ + update: &payload.Payload{ Manifests: []lib.Manifest{ { GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, @@ -229,7 +229,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) { }, { name: "requires verifiers", - update: &payload.Update{ + update: &payload.Payload{ Manifests: []lib.Manifest{ { GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, @@ -254,7 +254,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) { }, { name: "loads valid configuration", - update: &payload.Update{ + update: &payload.Payload{ Manifests: []lib.Manifest{ { GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, @@ -281,7 +281,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) { }, { name: "only the first valid configuration is used", - update: &payload.Update{ + update: &payload.Payload{ Manifests: []lib.Manifest{ { GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},