Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Commit

Permalink
Update rolloutblockerror to be consistent with other errors (#397)
Browse files Browse the repository at this point in the history
Co-authored-by: Panagiotis Nikoloutsopoulos <panagiotis.nikoloutsopoulos@booking.com>
  • Loading branch information
nikoloup and Panagiotis Nikoloutsopoulos authored May 19, 2021
1 parent 4c66fd4 commit c922bcf
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
12 changes: 8 additions & 4 deletions pkg/controller/application/application_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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,
Expand All @@ -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()
Expand All @@ -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)

Expand All @@ -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,
},
{
Expand All @@ -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()
Expand Down
9 changes: 5 additions & 4 deletions pkg/controller/release/release_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 8 additions & 6 deletions pkg/errors/rolloutblock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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}
}

0 comments on commit c922bcf

Please sign in to comment.