diff --git a/src/api/v1beta1/dynakube_status.go b/src/api/v1beta1/dynakube_status.go index ee445f2203..e1a3c7bf2c 100644 --- a/src/api/v1beta1/dynakube_status.go +++ b/src/api/v1beta1/dynakube_status.go @@ -124,11 +124,3 @@ func (dk *DynaKubeStatus) SetPhase(phase DynaKubePhaseType) bool { dk.Phase = phase return upd } - -// SetPhaseOnError fills the phase with the Error value in case of any error -func (dk *DynaKubeStatus) SetPhaseOnError(err error) bool { - if err != nil { - return dk.SetPhase(Error) - } - return false -} diff --git a/src/controllers/dynakube/activegate/internal/authtoken/reconciler.go b/src/controllers/dynakube/activegate/internal/authtoken/reconciler.go index c18122b804..a89de31372 100644 --- a/src/controllers/dynakube/activegate/internal/authtoken/reconciler.go +++ b/src/controllers/dynakube/activegate/internal/authtoken/reconciler.go @@ -46,7 +46,7 @@ func NewReconciler(clt client.Client, apiReader client.Reader, scheme *runtime.S func (r *Reconciler) Reconcile() error { err := r.reconcileAuthTokenSecret() if err != nil { - return errors.Errorf("failed to create activeGateAuthToken secret: %v", err) + return errors.WithMessage(err, "failed to create activeGateAuthToken secret") } return nil } @@ -77,7 +77,7 @@ func (r *Reconciler) reconcileAuthTokenSecret() error { func (r *Reconciler) ensureAuthTokenSecret() error { agSecretData, err := r.getActiveGateAuthToken() if err != nil { - return errors.Errorf("failed to create secret '%s': %v", r.dynakube.ActiveGateAuthTokenSecret(), err) + return errors.WithMessagef(err, "failed to create secret '%s'", r.dynakube.ActiveGateAuthTokenSecret()) } return r.createSecret(agSecretData) } diff --git a/src/controllers/dynakube/dynakube_controller.go b/src/controllers/dynakube/dynakube_controller.go index 661d2a5f30..7d519470f6 100644 --- a/src/controllers/dynakube/dynakube_controller.go +++ b/src/controllers/dynakube/dynakube_controller.go @@ -124,9 +124,9 @@ func (controller *Controller) Reconcile(ctx context.Context, request reconcile.R var serverErr dtclient.ServerError isServerError := errors.As(err, &serverErr) - if isServerError && serverErr.Code == http.StatusTooManyRequests { + if isServerError && (serverErr.Code == http.StatusTooManyRequests || serverErr.Code == http.StatusServiceUnavailable) { // should we set the phase to error ? - log.Info("request limit for Dynatrace API reached! Next reconcile in one minute") + log.Info("server is unavailable or request limit reached! trying again in one minute") return reconcile.Result{RequeueAfter: requeueAfter}, nil } dynakube.Status.SetPhase(dynatracev1beta1.Error) diff --git a/src/controllers/dynakube/dynakube_controller_test.go b/src/controllers/dynakube/dynakube_controller_test.go index c4ac1a39e1..1106f6e44b 100644 --- a/src/controllers/dynakube/dynakube_controller_test.go +++ b/src/controllers/dynakube/dynakube_controller_test.go @@ -3,6 +3,7 @@ package dynakube import ( "context" "fmt" + "net/http" "testing" dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1" @@ -990,6 +991,49 @@ func TestTokenConditions(t *testing.T) { }) } +func TestAPIError(t *testing.T) { + mockClient := createDTMockClient(dtclient.TokenScopes{dtclient.TokenScopeInstallerDownload}, + dtclient.TokenScopes{dtclient.TokenScopeDataExport, dtclient.TokenScopeActiveGateTokenCreate}, + ) + instance := &dynatracev1beta1.DynaKube{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + }, + Spec: dynatracev1beta1.DynaKubeSpec{ + APIURL: testApiUrl, + OneAgent: dynatracev1beta1.OneAgentSpec{CloudNativeFullStack: &dynatracev1beta1.CloudNativeFullStackSpec{HostInjectSpec: dynatracev1beta1.HostInjectSpec{}}}, + ActiveGate: dynatracev1beta1.ActiveGateSpec{ + Capabilities: []dynatracev1beta1.CapabilityDisplayName{ + dynatracev1beta1.KubeMonCapability.DisplayName, + }, + }, + }, + } + t.Run("should return error result on 503", func(t *testing.T) { + mockClient.On("GetActiveGateAuthToken", testName).Return(&dtclient.ActiveGateAuthTokenInfo{}, dtclient.ServerError{Code: http.StatusServiceUnavailable, Message: "Service unavailable"}) + controller := createFakeClientAndReconciler(mockClient, instance, testPaasToken, testAPIToken) + + result, err := controller.Reconcile(context.TODO(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: testName}, + }) + + assert.NoError(t, err) + assert.Equal(t, errorUpdateInterval, result.RequeueAfter) + }) + t.Run("should return error result on 429", func(t *testing.T) { + mockClient.On("GetActiveGateAuthToken", testName).Return(&dtclient.ActiveGateAuthTokenInfo{}, dtclient.ServerError{Code: http.StatusTooManyRequests, Message: "Too many requests"}) + controller := createFakeClientAndReconciler(mockClient, instance, testPaasToken, testAPIToken) + + result, err := controller.Reconcile(context.TODO(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: testName}, + }) + + assert.NoError(t, err) + assert.Equal(t, errorUpdateInterval, result.RequeueAfter) + }) +} + func assertCondition(t *testing.T, dk *dynatracev1beta1.DynaKube, expectedConditionType string, expectedConditionStatus metav1.ConditionStatus, expectedReason string, expectedMessage string) { //nolint:revive // argument-limit t.Helper() diff --git a/src/controllers/dynakube/oneagent/oneagent_reconciler_test.go b/src/controllers/dynakube/oneagent/oneagent_reconciler_test.go index fed4a641fc..b3d91ee0c5 100644 --- a/src/controllers/dynakube/oneagent/oneagent_reconciler_test.go +++ b/src/controllers/dynakube/oneagent/oneagent_reconciler_test.go @@ -12,7 +12,6 @@ import ( "github.com/Dynatrace/dynatrace-operator/src/scheme" "github.com/Dynatrace/dynatrace-operator/src/scheme/fake" "github.com/Dynatrace/dynatrace-operator/src/version" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -121,21 +120,6 @@ func TestReconcile_PhaseSetCorrectly(t *testing.T) { Reason: dynatracev1beta1.ReasonTokenReady, Message: "Ready", }) - - t.Run("SetPhaseOnError called with different values, object and return value correctly modified", func(t *testing.T) { - dk := base.DeepCopy() - - res := dk.Status.SetPhaseOnError(nil) - assert.False(t, res) - assert.Equal(t, dk.Status.Phase, dynatracev1beta1.DynaKubePhaseType("")) - - res = dk.Status.SetPhaseOnError(errors.New("dummy error")) - assert.True(t, res) - - if assert.NotNil(t, dk.Status.Phase) { - assert.Equal(t, dynatracev1beta1.Error, dk.Status.Phase) - } - }) } func TestReconcile_InstancesSet(t *testing.T) { diff --git a/src/controllers/dynakube/version/activegate.go b/src/controllers/dynakube/version/activegate.go index 9e348de0dc..2ade90da7d 100644 --- a/src/controllers/dynakube/version/activegate.go +++ b/src/controllers/dynakube/version/activegate.go @@ -64,5 +64,5 @@ func (updater *activeGateUpdater) CheckForDowngrade(latestVersion string) (bool, func (updater *activeGateUpdater) UseTenantRegistry(ctx context.Context, dockerCfg *dockerconfig.DockerConfig) error { defaultImage := updater.dynakube.DefaultActiveGateImage() - return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg) + return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg, updater.dynakube) } diff --git a/src/controllers/dynakube/version/oneagent.go b/src/controllers/dynakube/version/oneagent.go index 363305fe1b..5875a1b368 100644 --- a/src/controllers/dynakube/version/oneagent.go +++ b/src/controllers/dynakube/version/oneagent.go @@ -74,7 +74,7 @@ func (updater *oneAgentUpdater) UseTenantRegistry(ctx context.Context, dockerCfg } defaultImage := updater.dynakube.DefaultOneAgentImage() - return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg) + return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg, updater.dynakube) } func (updater *oneAgentUpdater) CheckForDowngrade(latestVersion string) (bool, error) { diff --git a/src/controllers/dynakube/version/synthetic.go b/src/controllers/dynakube/version/synthetic.go index b6fe896a16..b851877855 100644 --- a/src/controllers/dynakube/version/synthetic.go +++ b/src/controllers/dynakube/version/synthetic.go @@ -65,5 +65,5 @@ func (updater syntheticUpdater) CheckForDowngrade(latestVersion string) (bool, e func (updater *syntheticUpdater) UseTenantRegistry(ctx context.Context, dockerCfg *dockerconfig.DockerConfig) error { defaultImage := updater.dynakube.DefaultSyntheticImage() - return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg) + return updateVersionStatusForTenantRegistry(ctx, updater.Target(), defaultImage, updater.versionFunc, dockerCfg, updater.dynakube) } diff --git a/src/controllers/dynakube/version/updater.go b/src/controllers/dynakube/version/updater.go index 666db46b6e..b585711270 100644 --- a/src/controllers/dynakube/version/updater.go +++ b/src/controllers/dynakube/version/updater.go @@ -40,7 +40,7 @@ func (reconciler *Reconciler) run(ctx context.Context, updater versionStatusUpda customImage := updater.CustomImage() if customImage != "" { log.Info("updating version status according to custom image", "updater", updater.Name()) - err = setImageIDWithDigest(ctx, updater.Target(), customImage, reconciler.versionFunc, dockerCfg) + err = setImageIDWithDigest(ctx, updater.Target(), customImage, reconciler.versionFunc, dockerCfg, reconciler.dynakube) return err } @@ -68,7 +68,7 @@ func (reconciler *Reconciler) run(ctx context.Context, updater versionStatusUpda return err } - err = setImageIDWithDigest(ctx, updater.Target(), publicImage.String(), reconciler.versionFunc, dockerCfg) + err = setImageIDWithDigest(ctx, updater.Target(), publicImage.String(), reconciler.versionFunc, dockerCfg, reconciler.dynakube) if err != nil { log.Info("could not update version status according to the public registry", "updater", updater.Name()) return err @@ -94,12 +94,13 @@ func determineSource(updater versionStatusUpdater) dynatracev1beta1.VersionSourc return dynatracev1beta1.TenantRegistryVersionSource } -func setImageIDWithDigest( +func setImageIDWithDigest( //nolint:revive ctx context.Context, target *dynatracev1beta1.VersionStatus, imageUri string, imageVersionFunc ImageVersionFunc, dockerCfg *dockerconfig.DockerConfig, + dynakube *dynatracev1beta1.DynaKube, ) error { ref, err := reference.Parse(imageUri) if err != nil { @@ -115,8 +116,12 @@ func setImageIDWithDigest( } else if taggedRef, ok := ref.(reference.NamedTagged); ok { imageVersion, err := imageVersionFunc(ctx, imageUri, dockerCfg) if err != nil { + if !dynakube.HasProxy() { + log.Info("failed to determine image version") + return err + } target.ImageID = taggedRef.String() - log.Error(err, "failed to get image digest, falling back to tag") + log.Info("failed to determine image version because of proxy, falling back to tag") } else { canonRef, err := reference.WithDigest(taggedRef, imageVersion.Digest) if err != nil { @@ -139,12 +144,13 @@ func setImageIDWithDigest( return nil } -func updateVersionStatusForTenantRegistry( +func updateVersionStatusForTenantRegistry( //nolint:revive ctx context.Context, target *dynatracev1beta1.VersionStatus, imageUri string, imageVersionFunc ImageVersionFunc, dockerCfg *dockerconfig.DockerConfig, + dynakube *dynatracev1beta1.DynaKube, ) error { ref, err := reference.Parse(imageUri) if err != nil { @@ -159,7 +165,11 @@ func updateVersionStatusForTenantRegistry( if taggedRef, ok := ref.(reference.NamedTagged); ok { imageVersion, err := imageVersionFunc(ctx, imageUri, dockerCfg) if err != nil { - log.Error(err, "failed to determine image version, ignoring version") + if !dynakube.HasProxy() { + log.Info("failed to determine image version") + return err + } + log.Info("failed to determine image version because of proxy, ignoring version") } target.ImageID = taggedRef.String() target.Version = imageVersion.Version diff --git a/src/controllers/dynakube/version/updater_test.go b/src/controllers/dynakube/version/updater_test.go index 2dcb661d57..a0ecf96b6c 100644 --- a/src/controllers/dynakube/version/updater_test.go +++ b/src/controllers/dynakube/version/updater_test.go @@ -297,12 +297,20 @@ func TestUpdateVersionStatus(t *testing.T) { } testDockerCfg := &dockerconfig.DockerConfig{} - t.Run("failing to get digest should not cause error, should fall back to using the tag", func(t *testing.T) { + t.Run("failing to get digest should cause error", func(t *testing.T) { registry := newEmptyFakeRegistry() target := dynatracev1beta1.VersionStatus{} - err := setImageIDWithDigest(ctx, &target, testImage.String(), registry.ImageVersionExt, testDockerCfg) + err := setImageIDWithDigest(ctx, &target, testImage.String(), registry.ImageVersionExt, testDockerCfg, newClassicFullStackDynakube()) + assert.Error(t, err) + }) + + t.Run("failing to get digest should not cause error if proxy is set", func(t *testing.T) { + registry := newEmptyFakeRegistry() + target := dynatracev1beta1.VersionStatus{} + dynakube := newClassicFullStackDynakube() + dynakube.Spec.Proxy = &dynatracev1beta1.DynaKubeProxy{Value: "http://username:password@host:port"} + err := setImageIDWithDigest(ctx, &target, testImage.String(), registry.ImageVersionExt, testDockerCfg, dynakube) assert.NoError(t, err) - assert.Equal(t, testImage.String(), target.ImageID) }) t.Run("set status", func(t *testing.T) { @@ -312,7 +320,7 @@ func TestUpdateVersionStatus(t *testing.T) { }, }) target := dynatracev1beta1.VersionStatus{} - err := setImageIDWithDigest(ctx, &target, testImage.String(), registry.ImageVersionExt, testDockerCfg) + err := setImageIDWithDigest(ctx, &target, testImage.String(), registry.ImageVersionExt, testDockerCfg, newClassicFullStackDynakube()) require.NoError(t, err) assertVersionStatusEquals(t, registry, getTaggedReference(t, testImage.String()), target) }) @@ -326,7 +334,7 @@ func TestUpdateVersionStatus(t *testing.T) { t.Error("digest function was called unexpectedly") return ImageVersion{}, nil } - err := setImageIDWithDigest(ctx, &target, expectedID, boomFunc, testDockerCfg) + err := setImageIDWithDigest(ctx, &target, expectedID, boomFunc, testDockerCfg, newClassicFullStackDynakube()) require.NoError(t, err) assert.Equal(t, expectedID, target.ImageID) }) @@ -339,7 +347,7 @@ func TestUpdateVersionStatus(t *testing.T) { t.Error("digest function was called unexpectedly") return ImageVersion{}, nil } - err := setImageIDWithDigest(ctx, &target, expectedID, boomFunc, testDockerCfg) + err := setImageIDWithDigest(ctx, &target, expectedID, boomFunc, testDockerCfg, newClassicFullStackDynakube()) require.NoError(t, err) assert.Equal(t, expectedID, target.ImageID) })