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.

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

[service/route] allow updating labels and services #432

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Jan 8, 2024

with 18d0ea2 we introduced merging annotations and labels. It was required as other components can add annotations to a created service:

Author: Martin Schuppert <mschuppert@redhat.com>
Date:   Wed Sep 27 09:06:26 2023 +0200

    Merge Annotations and Labels in CreateOrPatch

    Identified in OCP4.13 where metallb adds an annotation to the
    service when handling it, it resulted in reconcile loop since
    the current service CreateOrPatch did not expect that annotations
    will be altered outside the owner.

    This changes the different functions to merge Labels and Annotations
    in CreateOr Patch instead of assigning.

With the current order of how the labels on routes and services get created does not allow to change a value of an existing key as how the merge func works.

This changes the order that existing labels and annotations of services and routes get merged so that those from an existing object gets merged into the one either provided via overrides or calculated by the operators. This still allows other components to add entries, and the operators/users to updated existing ones.

with 18d0ea2 we introduced merging
annotations and labels. It was required as other components can add
annotations to a created service:

~~~
Author: Martin Schuppert <mschuppert@redhat.com>
Date:   Wed Sep 27 09:06:26 2023 +0200

    Merge Annotations and Labels in CreateOrPatch

    Identified in OCP4.13 where metallb adds an annotation to the
    service when handling it, it resulted in reconcile loop since
    the current service CreateOrPatch did not expect that annotations
    will be altered outside the owner.

    This changes the different functions to merge Labels and Annotations
    in CreateOr Patch instead of assigning.
~~~

With the current order of how the labels on routes and services
get created does not allow to change a value of an existing
key as how the merge func works.

This changes the order that existing labels and annotations of
services and routes get merged so that those from an existing object
gets merged into the one either provided via overrides or calculated
by the operators. This still allows other components to add entries,
and the operators/users to updated existing ones.
@stuggi stuggi requested review from olliewalsh, abays and dprince January 8, 2024 14:25
stuggi added a commit to stuggi/glance-operator that referenced this pull request Jan 8, 2024
A route should get created for the glance-api instance which has
- annotation with glance.KeystoneEndpoint -> true
- it is the service.EndpointPublic
- and the k8s service is corev1.ServiceTypeClusterIP

Right now the service.AnnotationIngressCreateKey: "true" Annotation
gets set on all glance-api services.

Depends-On: openstack-k8s-operators/lib-common#432
Copy link
Contributor

@olliewalsh olliewalsh left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 3f12c32 into openstack-k8s-operators:main Jan 8, 2024
2 checks passed
stuggi added a commit to stuggi/glance-operator that referenced this pull request Jan 8, 2024
A route should get created for the glance-api instance which has
- annotation with glance.KeystoneEndpoint -> true
- it is the service.EndpointPublic
- and the k8s service is corev1.ServiceTypeClusterIP

Right now the service.AnnotationIngressCreateKey: "true" Annotation
gets set on all glance-api services.

Depends-On: openstack-k8s-operators/lib-common#432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants