Skip to content

Commit

Permalink
extract processing of one configuration resource into separate functi…
Browse files Browse the repository at this point in the history 
…on (#577)

This more of less only moves inner loop into a separate function. Little easier to think about and maybe opens up some opportunity to combine the ReconcileStackConfigMap and ReconcileStackSecret functions since they are very similar.

---------

Signed-off-by: Katyanna Moura <[email protected]>
Co-authored-by: Katyanna Moura <[email protected]>
  • Loading branch information
@linki @katyanna
linki and katyanna authored Feb 28, 2024
1 parent c8edb19 commit 6521cbd
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 60 deletions.
149 changes: 95 additions & 54 deletions controller/stack_resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
networking "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)

func pint32Equal(p1, p2 *int32) bool {
Expand All @@ -31,6 +32,15 @@ func syncObjectMeta(target, source metav1.Object) {
target.SetAnnotations(source.GetAnnotations())
}

// isOwned checks if the resource is owned and returns the UID of the owner.
func isOwned(ownerReferences []metav1.OwnerReference) (bool, types.UID) {
for _, ownerRef := range ownerReferences {
return true, ownerRef.UID
}

return false, ""
}

func (c *StackSetController) ReconcileStackDeployment(ctx context.Context, stack *zv1.Stack, existing *apps.Deployment, generateUpdated func() *apps.Deployment) error {
deployment := generateUpdated()

Expand Down Expand Up @@ -306,7 +316,7 @@ func (c *StackSetController) ReconcileStackRouteGroup(ctx context.Context, stack
return nil
}

// ReconcileStackConfigMapRefs will update the named user-provided ConfigMap to be
// ReconcileStackConfigMapRefs will update the named user-provided ConfigMaps to be
// attached to the Stack by ownerReferences, when a list of Configuration
// Resources are defined on the Stack template.
//
Expand All @@ -321,7 +331,6 @@ func (c *StackSetController) ReconcileStackRouteGroup(ctx context.Context, stack
func (c *StackSetController) ReconcileStackConfigMapRefs(
ctx context.Context,
stack *zv1.Stack,
existing []*apiv1.ConfigMap,
updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta,
) error {
for _, rsc := range stack.Spec.ConfigurationResources {
Expand All @@ -333,42 +342,59 @@ func (c *StackSetController) ReconcileStackConfigMapRefs(
return err
}

configMap, err := c.client.CoreV1().ConfigMaps(stack.Namespace).
Get(ctx, rsc.GetName(), metav1.GetOptions{})
if err != nil {
if err := c.ReconcileStackConfigMapRef(ctx, stack, rsc, updateObjMeta); err != nil {
return err
}
}

if configMap.OwnerReferences != nil {
for _, owner := range configMap.OwnerReferences {
if owner.UID != stack.UID {
return fmt.Errorf("ConfigMap already owned by other resource. "+
"ConfigMap: %s, Stack: %s", rsc.GetName(), stack.Name)
}
}
continue
}
return nil
}

objectMeta := updateObjMeta(&configMap.ObjectMeta)
configMap.ObjectMeta = *objectMeta
func (c *StackSetController) ReconcileStackConfigMapRef(
ctx context.Context,
stack *zv1.Stack,
rsc zv1.ConfigurationResourcesSpec,
updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta,
) error {
configMap, err := c.client.CoreV1().ConfigMaps(stack.Namespace).
Get(ctx, rsc.GetName(), metav1.GetOptions{})
if err != nil {
return err
}

_, err = c.client.CoreV1().ConfigMaps(configMap.Namespace).
Update(ctx, configMap, metav1.UpdateOptions{})
if err != nil {
return err
// Check if the ConfigMap is already owned by us or another resource.
// If the ConfigMap is owned by another resource, we should not update it.
isOwned, owner := isOwned(configMap.OwnerReferences)
if isOwned {
if owner == stack.UID {
return nil
}
c.recorder.Eventf(
stack,
apiv1.EventTypeNormal,
"UpdatedConfigMap",
"Updated ConfigMap %s",
configMap.Name,
)

return fmt.Errorf("ConfigMap already owned by other resource. "+
"ConfigMap: %s, Stack: %s", rsc.GetName(), stack.Name)
}

objectMeta := updateObjMeta(&configMap.ObjectMeta)
configMap.ObjectMeta = *objectMeta

_, err = c.client.CoreV1().ConfigMaps(configMap.Namespace).
Update(ctx, configMap, metav1.UpdateOptions{})
if err != nil {
return err
}

c.recorder.Eventf(
stack,
apiv1.EventTypeNormal,
"UpdatedConfigMap",
"Updated ConfigMap %s",
configMap.Name,
)

return nil
}

// ReconcileStackSecretRefs will update the named user-provided Secret to be
// ReconcileStackSecretRefs will update the named user-provided Secrets to be
// attached to the Stack by ownerReferences, when a list of Configuration
// Resources are defined on the Stack template.
//
Expand All @@ -383,7 +409,6 @@ func (c *StackSetController) ReconcileStackConfigMapRefs(
func (c *StackSetController) ReconcileStackSecretRefs(
ctx context.Context,
stack *zv1.Stack,
existing []*apiv1.Secret,
updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta,
) error {
for _, rsc := range stack.Spec.ConfigurationResources {
Expand All @@ -395,37 +420,53 @@ func (c *StackSetController) ReconcileStackSecretRefs(
return err
}

secret, err := c.client.CoreV1().Secrets(stack.Namespace).
Get(ctx, rsc.GetName(), metav1.GetOptions{})
if err != nil {
if err := c.ReconcileStackSecretRef(ctx, stack, rsc, updateObjMeta); err != nil {
return err
}
}

if secret.OwnerReferences != nil {
for _, owner := range secret.OwnerReferences {
if owner.UID != stack.UID {
return fmt.Errorf("Secret already owned by other resource. "+
"Secret: %s, Stack: %s", rsc.GetName(), stack.Name)
}
}
continue
}
return nil
}

objectMeta := updateObjMeta(&secret.ObjectMeta)
secret.ObjectMeta = *objectMeta
func (c *StackSetController) ReconcileStackSecretRef(ctx context.Context,
stack *zv1.Stack,
rsc zv1.ConfigurationResourcesSpec,
updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta,
) error {
secret, err := c.client.CoreV1().Secrets(stack.Namespace).
Get(ctx, rsc.GetName(), metav1.GetOptions{})
if err != nil {
return err
}

_, err = c.client.CoreV1().Secrets(secret.Namespace).
Update(ctx, secret, metav1.UpdateOptions{})
if err != nil {
return err
// Check if the Secret is already owned by us or another resource.
// If the Secret is owned by another resource, we should not update it.
isOwned, owner := isOwned(secret.OwnerReferences)
if isOwned {
if owner == stack.UID {
return nil
}
c.recorder.Eventf(
stack,
apiv1.EventTypeNormal,
"UpdatedSecret",
"Updated Secret %s",
secret.Name,
)

return fmt.Errorf("Secret already owned by other resource. "+
"Secret: %s, Stack: %s", rsc.GetName(), stack.Name)
}

objectMeta := updateObjMeta(&secret.ObjectMeta)
secret.ObjectMeta = *objectMeta

_, err = c.client.CoreV1().Secrets(secret.Namespace).
Update(ctx, secret, metav1.UpdateOptions{})
if err != nil {
return err
}

c.recorder.Eventf(
stack,
apiv1.EventTypeNormal,
"UpdatedSecret",
"Updated Secret %s",
secret.Name,
)

return nil
}
4 changes: 2 additions & 2 deletions controller/stack_resources_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ func TestReconcileStackConfigMapRefs(t *testing.T) {
}

err = env.controller.ReconcileStackConfigMapRefs(
context.Background(), &tc.stack, tc.existing, func(tmp *metav1.ObjectMeta) *metav1.ObjectMeta {
context.Background(), &tc.stack, func(tmp *metav1.ObjectMeta) *metav1.ObjectMeta {
return &tc.expected[tmp.Name].ObjectMeta
})
require.NoError(t, err)
Expand Down Expand Up @@ -1413,7 +1413,7 @@ func TestReconcileStackSecretRefs(t *testing.T) {
}

err = env.controller.ReconcileStackSecretRefs(
context.Background(), &tc.stack, tc.existing, func(tmp *metav1.ObjectMeta) *metav1.ObjectMeta {
context.Background(), &tc.stack, func(tmp *metav1.ObjectMeta) *metav1.ObjectMeta {
return &tc.expected[tmp.Name].ObjectMeta
})
require.NoError(t, err)
Expand Down
8 changes: 4 additions & 4 deletions controller/stackset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1321,14 +1321,14 @@ func (c *StackSetController) ReconcileStackResources(ctx context.Context, ssc *c
}

if c.configMapSupportEnabled {
err = c.ReconcileStackConfigMapRefs(ctx, sc.Stack, sc.Resources.ConfigMaps, sc.UpdateObjectMeta)
err := c.ReconcileStackConfigMapRefs(ctx, sc.Stack, sc.UpdateObjectMeta)
if err != nil {
return c.errorEventf(sc.Stack, "FailedManageConfigMapRefs", err)
}
}

if c.secretSupportEnabled {
err := c.ReconcileStackSecretRefs(ctx, sc.Stack, sc.Resources.Secrets, sc.UpdateObjectMeta)
err := c.ReconcileStackSecretRefs(ctx, sc.Stack, sc.UpdateObjectMeta)
if err != nil {
return c.errorEventf(sc.Stack, "FailedManageSecretRefs", err)
}
Expand Down Expand Up @@ -1523,8 +1523,8 @@ func resourceReadyTime(timestamp time.Time, ttl time.Duration) bool {
// name is not prefixed by Stack name.
func validateAllConfigurationResourcesNames(stack *zv1.Stack) error {
for _, rsc := range stack.Spec.ConfigurationResources {
if !strings.HasPrefix(rsc.GetName(), stack.Name) {
return fmt.Errorf(configurationResourceNameError, rsc.GetName(), stack.Name)
if err := validateConfigurationResourceName(stack.Name, rsc.GetName()); err != nil {
return err
}
}
return nil
Expand Down

0 comments on commit 6521cbd

Please sign in to comment.