Skip to content

Commit

Permalink
Swap hashi mutlierror for native go implementation (#1042)
Browse files Browse the repository at this point in the history
* Swap hashi mutlierror for native go implementation & Update TestSyncCustomRolesStatus to use new multierror output
  • Loading branch information
roothorp authored Jul 20, 2023
1 parent a408efc commit 698123b
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 27 deletions.
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/go-logr/zapr v1.2.4
github.com/google/go-cmp v0.5.9
github.com/google/uuid v1.3.0
github.com/hashicorp/go-multierror v1.1.1
github.com/mongodb-forks/digest v1.0.4
github.com/onsi/ginkgo/v2 v2.11.0
github.com/onsi/gomega v1.27.8
Expand Down Expand Up @@ -82,7 +81,6 @@ require (
github.com/google/gofuzz v1.2.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.2.5 // indirect
github.com/googleapis/gax-go/v2 v2.12.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,6 @@ github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5m
github.com/googleapis/gax-go/v2 v2.12.0 h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56etFpas=
github.com/googleapis/gax-go/v2 v2.12.0/go.mod h1:y+aIqrI5eb1YGMVJfuV3185Ts/D7qKpsEkdD5+I6QGU=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down
9 changes: 4 additions & 5 deletions pkg/controller/atlasproject/custom_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ package atlasproject

import (
"context"
"errors"
"fmt"

"github.com/hashicorp/go-multierror"

"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -240,7 +239,7 @@ func syncCustomRolesStatus(ctx *workflow.Context, desiredCustomRoles []v1.Custom
if ok {
if stat.Status == status.CustomRoleStatusFailed {
statuses = append(statuses, stat)
err = multierror.Append(err, fmt.Errorf(stat.Error))
err = errors.Join(err, fmt.Errorf(stat.Error))
}

continue
Expand All @@ -250,7 +249,7 @@ func syncCustomRolesStatus(ctx *workflow.Context, desiredCustomRoles []v1.Custom
statuses = append(statuses, stat)

if stat.Status == status.CustomRoleStatusFailed {
err = multierror.Append(err, fmt.Errorf(stat.Error))
err = errors.Join(err, fmt.Errorf(stat.Error))
}

continue
Expand All @@ -260,7 +259,7 @@ func syncCustomRolesStatus(ctx *workflow.Context, desiredCustomRoles []v1.Custom
statuses = append(statuses, stat)

if stat.Status == status.CustomRoleStatusFailed {
err = multierror.Append(err, fmt.Errorf(stat.Error))
err = errors.Join(err, fmt.Errorf(stat.Error))
}

continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/atlasproject/custom_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestSyncCustomRolesStatus(t *testing.T) {

assert.Equal(
t,
workflow.Terminate(workflow.ProjectCustomRolesReady, "failed to apply changes to custom roles: 1 error occurred:\n\t* server failed\n\n"),
workflow.Terminate(workflow.ProjectCustomRolesReady, "failed to apply changes to custom roles: server failed"),
syncCustomRolesStatus(ctx, desired, created, updated, deleted),
)

Expand Down
28 changes: 13 additions & 15 deletions pkg/controller/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/hashicorp/go-multierror"

mdbv1 "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1"
)

Expand All @@ -36,31 +34,31 @@ func DeploymentSpec(deploymentSpec mdbv1.AtlasDeploymentSpec) error {
var err error

if allAreNil(deploymentSpec.AdvancedDeploymentSpec, deploymentSpec.ServerlessSpec, deploymentSpec.DeploymentSpec) {
err = multierror.Append(err, errors.New("expected exactly one of spec.deploymentSpec or spec.advancedDepploymentSpec or spec.serverlessSpec to be present, but none were"))
err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec or spec.advancedDepploymentSpec or spec.serverlessSpec to be present, but none were"))
}

if moreThanOneIsNonNil(deploymentSpec.AdvancedDeploymentSpec, deploymentSpec.ServerlessSpec, deploymentSpec.DeploymentSpec) {
err = multierror.Append(err, errors.New("expected exactly one of spec.deploymentSpec, spec.advancedDepploymentSpec or spec.serverlessSpec, more than one were present"))
err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec, spec.advancedDepploymentSpec or spec.serverlessSpec, more than one were present"))
}

if deploymentSpec.DeploymentSpec != nil {
if deploymentSpec.DeploymentSpec.ProviderSettings != nil && (deploymentSpec.DeploymentSpec.ProviderSettings.InstanceSizeName == "" && deploymentSpec.DeploymentSpec.ProviderSettings.ProviderName != "SERVERLESS") {
err = multierror.Append(err, errors.New("must specify instanceSizeName if provider name is not SERVERLESS"))
err = errors.Join(err, errors.New("must specify instanceSizeName if provider name is not SERVERLESS"))
}
if deploymentSpec.DeploymentSpec.ProviderSettings != nil && (deploymentSpec.DeploymentSpec.ProviderSettings.InstanceSizeName != "" && deploymentSpec.DeploymentSpec.ProviderSettings.ProviderName == "SERVERLESS") {
err = multierror.Append(err, errors.New("must not specify instanceSizeName if provider name is SERVERLESS"))
err = errors.Join(err, errors.New("must not specify instanceSizeName if provider name is SERVERLESS"))
}
}

if deploymentSpec.AdvancedDeploymentSpec != nil {
instanceSizeErr := instanceSizeForAdvancedDeployment(deploymentSpec.AdvancedDeploymentSpec.ReplicationSpecs)
if instanceSizeErr != nil {
err = multierror.Append(err, instanceSizeErr)
err = errors.Join(err, instanceSizeErr)
}

autoscalingErr := autoscalingForAdvancedDeployment(deploymentSpec.AdvancedDeploymentSpec.ReplicationSpecs)
if autoscalingErr != nil {
err = multierror.Append(err, autoscalingErr)
err = errors.Join(err, autoscalingErr)
}
}

Expand All @@ -87,7 +85,7 @@ func BackupSchedule(bSchedule *mdbv1.AtlasBackupSchedule, deployment *mdbv1.Atla
var err error

if bSchedule.Spec.Export == nil && bSchedule.Spec.AutoExportEnabled {
err = multierror.Append(err, errors.New("you must specify export policy when auto export is enabled"))
err = errors.Join(err, errors.New("you must specify export policy when auto export is enabled"))
}

replicaSets := map[string]struct{}{}
Expand All @@ -99,26 +97,26 @@ func BackupSchedule(bSchedule *mdbv1.AtlasBackupSchedule, deployment *mdbv1.Atla

for position, copySetting := range bSchedule.Spec.CopySettings {
if copySetting.RegionName == nil {
err = multierror.Append(err, fmt.Errorf("copy setting at position %d: you must set a region name", position))
err = errors.Join(err, fmt.Errorf("copy setting at position %d: you must set a region name", position))
}

if copySetting.ReplicationSpecID == nil {
err = multierror.Append(err, fmt.Errorf("copy setting at position %d: you must set a valid ReplicationSpecID", position))
err = errors.Join(err, fmt.Errorf("copy setting at position %d: you must set a valid ReplicationSpecID", position))
} else if _, ok := replicaSets[*copySetting.ReplicationSpecID]; !ok {
err = multierror.Append(err, fmt.Errorf("copy setting at position %d: referenced ReplicationSpecID is invalid", position))
err = errors.Join(err, fmt.Errorf("copy setting at position %d: referenced ReplicationSpecID is invalid", position))
}

if copySetting.ShouldCopyOplogs != nil && *copySetting.ShouldCopyOplogs {
if deployment.Spec.AdvancedDeploymentSpec != nil &&
(deployment.Spec.AdvancedDeploymentSpec.PitEnabled == nil ||
!*deployment.Spec.AdvancedDeploymentSpec.PitEnabled) {
err = multierror.Append(err, fmt.Errorf("copy setting at position %d: you must enable pit before enable copyOplogs", position))
err = errors.Join(err, fmt.Errorf("copy setting at position %d: you must enable pit before enable copyOplogs", position))
}

if deployment.Spec.DeploymentSpec != nil &&
(deployment.Spec.DeploymentSpec.PitEnabled == nil ||
!*deployment.Spec.DeploymentSpec.PitEnabled) {
err = multierror.Append(err, fmt.Errorf("copy setting at position %d: you must enable pit before enable copyOplogs", position))
err = errors.Join(err, fmt.Errorf("copy setting at position %d: you must enable pit before enable copyOplogs", position))
}
}
}
Expand Down Expand Up @@ -211,7 +209,7 @@ func projectCustomRoles(customRoles []mdbv1.CustomRole) error {

for _, customRole := range customRoles {
if _, ok := customRolesMap[customRole.Name]; ok {
err = multierror.Append(err, fmt.Errorf("the custom rone \"%s\" is duplicate. custom role name must be unique", customRole.Name))
err = errors.Join(err, fmt.Errorf("the custom rone \"%s\" is duplicate. custom role name must be unique", customRole.Name))
}

customRolesMap[customRole.Name] = struct{}{}
Expand Down

0 comments on commit 698123b

Please sign in to comment.