Merge annotations from desired with existing ones in the ServiceAccounts

[rubenvp8510]

Fixes #970

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.36%. Comparing base (76bfab6) to head (8b09d44).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #969      +/-   ##
==========================================
- Coverage   73.36%   73.36%   -0.01%     
==========================================
  Files         105      105              
  Lines        6488     6487       -1     
==========================================
- Hits         4760     4759       -1     
  Misses       1438     1438              
  Partials      290      290              

Flag	Coverage Δ
unittests	73.36% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rubenvp8510]

@andreasgerstmayr This PR only verifies if the SA exists or not in order to determine if needs to be recreated. Do you think we need a more sophisticated way to validate this? Because currently our SAs are very simple.

[rubenvp8510]

Tested on OCP 4.16

➜  reconcile git:(sa_fix) ✗ oc get pods -n ruben-test
NAME                                             READY   STATUS    RESTARTS   AGE
minio-687794c4f-6g45v                            1/1     Running   0          7h25m
tempo-simplest-compactor-7b9cd9d4dd-jn7jg        1/1     Running   0          77s
tempo-simplest-distributor-8584d97bc6-wkcnp      1/1     Running   0          77s
tempo-simplest-ingester-0                        1/1     Running   0          77s
tempo-simplest-querier-7fd7d8b76f-nrfcw          1/1     Running   0          77s
tempo-simplest-query-frontend-6c79bbb7d7-96q6c   1/1     Running   0          77s
➜  reconcile git:(sa_fix) ✗ oc get secrets -n ruben-test
NAME                                 TYPE                      DATA   AGE
builder-dockercfg-dcvmn              kubernetes.io/dockercfg   1      7h45m
default-dockercfg-mj4wx              kubernetes.io/dockercfg   1      7h45m
deployer-dockercfg-h5szn             kubernetes.io/dockercfg   1      7h45m
minio-test                           Opaque                    4      7h45m
tempo-simplest-compactor-mtls        kubernetes.io/tls         2      21m
tempo-simplest-distributor-mtls      kubernetes.io/tls         2      21m
tempo-simplest-dockercfg-xdwgm       kubernetes.io/dockercfg   1      21m
tempo-simplest-gateway-mtls          kubernetes.io/tls         2      21m
tempo-simplest-ingester-mtls         kubernetes.io/tls         2      21m
tempo-simplest-querier-mtls          kubernetes.io/tls         2      21m
tempo-simplest-query-frontend-mtls   kubernetes.io/tls         2      21m
tempo-simplest-signing-ca            kubernetes.io/tls         2      21m
test-sa-dockercfg-5cdlk              kubernetes.io/dockercfg   1      3h23m
➜  reconcile git:(sa_fix) ✗ 

[iblancasa]

Great work!

[andreasgerstmayr]

@andreasgerstmayr This PR only verifies if the SA exists or not in order to determine if needs to be recreated. Do you think we need a more sophisticated way to validate this? Because currently our SAs are very simple.

I think for this PR we only need to remove this line:

tempo-operator/internal/manifests/mutate.go

Line 184 in 76bfab6

existing.Annotations = desired.Annotations

then the issue will be resolved if I'm not mistaken.

Or something like

tempo-operator/internal/manifests/mutate.go

Line 172 in 76bfab6

if _, ok := desired.Annotations["service.beta.openshift.io/inject-cabundle"]; ok {

to be more sophisticated :D

[rubenvp8510]

@andreasgerstmayr This PR only verifies if the SA exists or not in order to determine if needs to be recreated. Do you think we need a more sophisticated way to validate this? Because currently our SAs are very simple.

I think for this PR we only need to remove this line:

tempo-operator/internal/manifests/mutate.go

Line 184 in 76bfab6

existing.Annotations = desired.Annotations

then the issue will be resolved if I'm not mistaken.

Or something like

tempo-operator/internal/manifests/mutate.go

Line 172 in 76bfab6

if _, ok := desired.Annotations["service.beta.openshift.io/inject-cabundle"]; ok {

to be more sophisticated :D

Not sure if that will work, I think other things are added to the SA not just the annotation. Anyway I'm gonna check this in the morning. If this simple approach work I don't see why not using it.

[rubenvp8510]

Ill switch this solution to use those mutation functions that @andreasgerstmayr is mentioning.

This PR is still rudimentary because it is only for demostrate the problem

[rubenvp8510]

@andreasgerstmayr This PR only verifies if the SA exists or not in order to determine if needs to be recreated. Do you think we need a more sophisticated way to validate this? Because currently our SAs are very simple.

I think for this PR we only need to remove this line:

tempo-operator/internal/manifests/mutate.go

Line 184 in 76bfab6

existing.Annotations = desired.Annotations

then the issue will be resolved if I'm not mistaken.

This did the trick. :)

Or something like

tempo-operator/internal/manifests/mutate.go

Line 172 in 76bfab6

if _, ok := desired.Annotations["service.beta.openshift.io/inject-cabundle"]; ok {

to be more sophisticated :D