Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed the order of CreateOrUpdate functions #1623

Merged
merged 2 commits into from
Sep 30, 2024
+18 −14
Merged

Changed the order of CreateOrUpdate functions #1623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f79c843
Changed order or CreateOrUpdate functions
lsierant Sep 28, 2024
b456e5b
Fixed mockedclient to return NotFound on update
lsierant Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/kube/client/mocked_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ func (m mockedClient) Create(_ context.Context, obj k8sClient.Object, _ ...k8sCl
func (m mockedClient) Update(_ context.Context, obj k8sClient.Object, _ ...k8sClient.UpdateOption) error {
relevantMap := m.ensureMapFor(obj)
objKey := k8sClient.ObjectKeyFromObject(obj)
if _, ok := relevantMap[objKey]; !ok {
return errors.NewNotFound(schema.GroupResource{}, obj.GetName())
}
relevantMap[objKey] = obj
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kube/configmap/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ func UpdateField(ctx context.Context, getUpdater GetUpdater, objectKey client.Ob
// CreateOrUpdate creates the given ConfigMap if it doesn't exist,
// or updates it if it does.
func CreateOrUpdate(ctx context.Context, getUpdateCreator GetUpdateCreator, cm corev1.ConfigMap) error {
_, err := getUpdateCreator.GetConfigMap(ctx, types.NamespacedName{Name: cm.Name, Namespace: cm.Namespace})
if err != nil {
if err := getUpdateCreator.UpdateConfigMap(ctx, cm); err != nil {
if apiErrors.IsNotFound(err) {
return getUpdateCreator.CreateConfigMap(ctx, cm)
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am a bit late to the party but we could have saved the else and just return, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I see such if/else I always wonder if I should leave a comment
Should we try to be consistent in the whole codebase about that ?
We use both currently

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add a linter for that and we should be fine. Problem: mco doesn't have that linter configured

return err
}
return err
}
return getUpdateCreator.UpdateConfigMap(ctx, cm)
return nil
}

// filelikePropertiesToMap converts a file-like field in a ConfigMap to a map[string]string.
Expand Down
8 changes: 4 additions & 4 deletions pkg/kube/secret/secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ func UpdateField(ctx context.Context, getUpdater GetUpdater, objectKey client.Ob

// CreateOrUpdate creates the Secret if it doesn't exist, other wise it updates it
func CreateOrUpdate(ctx context.Context, getUpdateCreator GetUpdateCreator, secret corev1.Secret) error {
_, err := getUpdateCreator.GetSecret(ctx, types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace})
if err != nil {
if err := getUpdateCreator.UpdateSecret(ctx, secret); err != nil {
if SecretNotExist(err) {
return getUpdateCreator.CreateSecret(ctx, secret)
} else {
return err
}
return err
}
return getUpdateCreator.UpdateSecret(ctx, secret)
return nil
}

// HasAllKeys returns true if the provided secret contains an element for every
Expand Down
13 changes: 7 additions & 6 deletions pkg/kube/statefulset/statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,16 @@ type GetUpdateCreateDeleter interface {

// CreateOrUpdate creates the given StatefulSet if it doesn't exist,
// or updates it if it does.
func CreateOrUpdate(ctx context.Context, getUpdateCreator GetUpdateCreator, sts appsv1.StatefulSet) (appsv1.StatefulSet, error) {
_, err := getUpdateCreator.GetStatefulSet(ctx, types.NamespacedName{Name: sts.Name, Namespace: sts.Namespace})
if err != nil {
func CreateOrUpdate(ctx context.Context, getUpdateCreator GetUpdateCreator, statefulSet appsv1.StatefulSet) (appsv1.StatefulSet, error) {
if sts, err := getUpdateCreator.UpdateStatefulSet(ctx, statefulSet); err != nil {
if apiErrors.IsNotFound(err) {
return appsv1.StatefulSet{}, getUpdateCreator.CreateStatefulSet(ctx, sts)
return statefulSet, getUpdateCreator.CreateStatefulSet(ctx, statefulSet)
} else {
return appsv1.StatefulSet{}, err
}
return appsv1.StatefulSet{}, err
} else {
return sts, nil
}
return getUpdateCreator.UpdateStatefulSet(ctx, sts)
}

// GetAndUpdate applies the provided function to the most recent version of the object
Expand Down
Loading