diff --git a/pkg/controller/application/application_controller_test.go b/pkg/controller/application/application_controller_test.go index c96ee3de6..fd68a1fb9 100644 --- a/pkg/controller/application/application_controller_test.go +++ b/pkg/controller/application/application_controller_test.go @@ -273,6 +273,8 @@ func TestCreateFirstReleaseWithNonExistingRolloutBlockOverride(t *testing.T) { f := newFixture(t) app := newApplication(testAppName) app.Annotations[shipper.RolloutBlocksOverrideAnnotation] = fmt.Sprintf("%s/%s", shippertesting.TestNamespace, "test-non-existing-rolloutblock") + invalidRolloutBlockKey := "test-namespace/test-non-existing-rolloutblock" + invalidRolloutBlockMessage := shippererrors.NewInvalidRolloutBlockOverrideError(invalidRolloutBlockKey).Error() f.objects = append(f.objects, app) @@ -291,7 +293,7 @@ func TestCreateFirstReleaseWithNonExistingRolloutBlockOverride(t *testing.T) { Type: shipper.ApplicationConditionTypeBlocked, Status: corev1.ConditionTrue, Reason: shipper.RolloutBlockReason, - Message: "rollout block with name test-namespace/test-non-existing-rolloutblock does not exist", + Message: invalidRolloutBlockMessage, }, { Type: shipper.ApplicationConditionTypeReleaseSynced, @@ -312,7 +314,7 @@ func TestCreateFirstReleaseWithNonExistingRolloutBlockOverride(t *testing.T) { f.expectedEvents = []string{ fmt.Sprintf(`Normal ApplicationConditionChanged [] -> [Aborting False], [] -> [ValidHistory True], [] -> [ReleaseSynced True], [] -> [RollingOut Unknown no contender release found for application "%s"]`, app.Name), - `Normal ApplicationConditionChanged [] -> [Blocked True RolloutsBlocked rollout block with name test-namespace/test-non-existing-rolloutblock does not exist]`, + fmt.Sprintf(`Normal ApplicationConditionChanged [] -> [Blocked True RolloutsBlocked %s]`, invalidRolloutBlockMessage), } f.run() @@ -322,6 +324,8 @@ func TestBlockApplication(t *testing.T) { f := newFixture(t) rolloutblock := newRolloutBlock(testRolloutBlockName) + rolloutBlockKey := fmt.Sprintf("%s/%s", rolloutblock.Namespace, rolloutblock.Name) + rolloutBlockMessage := shippererrors.NewRolloutBlockError(rolloutBlockKey).Error() f.objects = append(f.objects, rolloutblock) app := newApplication(testAppName) @@ -340,7 +344,7 @@ func TestBlockApplication(t *testing.T) { { Type: shipper.ApplicationConditionTypeBlocked, Reason: shipper.RolloutBlockReason, - Message: fmt.Sprintf("rollout block(s) with name(s) %s/%s exist", rolloutblock.Namespace, rolloutblock.Name), + Message: rolloutBlockMessage, Status: corev1.ConditionTrue, }, { @@ -363,7 +367,7 @@ func TestBlockApplication(t *testing.T) { f.expectedEvents = []string{ fmt.Sprintf("Warning RolloutBlocked %s/%s", rolloutblock.Namespace, rolloutblock.Name), fmt.Sprintf(`Normal ApplicationConditionChanged [] -> [Aborting False], [] -> [ValidHistory True], [] -> [ReleaseSynced True], [] -> [RollingOut Unknown no contender release found for application "%s"]`, app.Name), - fmt.Sprintf(`Normal ApplicationConditionChanged [] -> [Blocked True RolloutsBlocked rollout block(s) with name(s) %s/%s exist]`, rolloutblock.Namespace, rolloutblock.Name), + fmt.Sprintf(`Normal ApplicationConditionChanged [] -> [Blocked True RolloutsBlocked %s]`, rolloutBlockMessage), } f.run() diff --git a/pkg/controller/release/release_controller_test.go b/pkg/controller/release/release_controller_test.go index 3178896c8..75c5a8e2b 100644 --- a/pkg/controller/release/release_controller_test.go +++ b/pkg/controller/release/release_controller_test.go @@ -22,6 +22,7 @@ import ( shipper "github.com/bookingcom/shipper/pkg/apis/shipper/v1alpha1" shipperfake "github.com/bookingcom/shipper/pkg/client/clientset/versioned/fake" shipperinformers "github.com/bookingcom/shipper/pkg/client/informers/externalversions" + shippererrors "github.com/bookingcom/shipper/pkg/errors" shippertesting "github.com/bookingcom/shipper/pkg/testing" apputil "github.com/bookingcom/shipper/pkg/util/application" "github.com/bookingcom/shipper/pkg/util/conditions" @@ -1789,7 +1790,7 @@ func TestContenderCapacityShouldNotIncreaseWithRolloutBlock(t *testing.T) { ) expectedContender := contender.release.DeepCopy() - rolloutBlockMessage := fmt.Sprintf("rollout block(s) with name(s) %s exist", rolloutBlockKey) + rolloutBlockMessage := shippererrors.NewRolloutBlockError(rolloutBlockKey).Error() condBlocked := releaseutil.NewReleaseCondition( shipper.ReleaseConditionTypeBlocked, corev1.ConditionTrue, @@ -1921,7 +1922,7 @@ func TestContenderTrafficShouldNotIncreaseWithRolloutBlock(t *testing.T) { ) expectedContender := contender.release.DeepCopy() - rolloutBlockMessage := fmt.Sprintf("rollout block(s) with name(s) %s exist", rolloutBlockKey) + rolloutBlockMessage := shippererrors.NewRolloutBlockError(rolloutBlockKey).Error() condBlocked := releaseutil.NewReleaseCondition( shipper.ReleaseConditionTypeBlocked, corev1.ConditionTrue, @@ -2048,7 +2049,7 @@ func TestIncumbentTrafficShouldNotDecreaseWithRolloutBlock(t *testing.T) { ) expectedContender := contender.release.DeepCopy() - rolloutBlockMessage := fmt.Sprintf("rollout block(s) with name(s) %s exist", rolloutBlockKey) + rolloutBlockMessage := shippererrors.NewRolloutBlockError(rolloutBlockKey).Error() condBlocked := releaseutil.NewReleaseCondition( shipper.ReleaseConditionTypeBlocked, corev1.ConditionTrue, @@ -2179,7 +2180,7 @@ func TestIncumbentCapacityShouldNotDecreaseWithRolloutBlock(t *testing.T) { ) expectedContender := contender.release.DeepCopy() - rolloutBlockMessage := fmt.Sprintf("rollout block(s) with name(s) %s exist", rolloutBlockKey) + rolloutBlockMessage := shippererrors.NewRolloutBlockError(rolloutBlockKey).Error() condBlocked := releaseutil.NewReleaseCondition( shipper.ReleaseConditionTypeBlocked, corev1.ConditionTrue, diff --git a/pkg/errors/rolloutblock.go b/pkg/errors/rolloutblock.go index 6bab5e3c9..20b05f65a 100644 --- a/pkg/errors/rolloutblock.go +++ b/pkg/errors/rolloutblock.go @@ -9,7 +9,7 @@ type InvalidRolloutBlockOverrideError struct { } func (e InvalidRolloutBlockOverrideError) Error() string { - return fmt.Sprintf("rollout block with name %s does not exist", + return fmt.Sprintf("rollout block with name %q does not exist", e.RolloutBlockName) } @@ -21,17 +21,19 @@ func NewInvalidRolloutBlockOverrideError(invalidRolloutBlockName string) Invalid return InvalidRolloutBlockOverrideError{invalidRolloutBlockName} } -type RolloutBlockError string +type RolloutBlockError struct { + RolloutBlockName string +} func (e RolloutBlockError) Error() string { - return string(e) + return fmt.Sprintf("rollout block(s) with name(s) %q exist", + e.RolloutBlockName) } func (e RolloutBlockError) ShouldRetry() bool { return false } -func NewRolloutBlockError(invalidRolloutBlockName string) RolloutBlockError { - return RolloutBlockError(fmt.Sprintf("rollout block(s) with name(s) %s exist", - invalidRolloutBlockName)) +func NewRolloutBlockError(rolloutBlockName string) RolloutBlockError { + return RolloutBlockError{rolloutBlockName} }