fix: use server-side-apply for all operations #80

kadel · 2024-08-15T12:13:45Z

Description

This changes all Patch and Create operations to use server-side apply.
This should lower risks of conflicts in situations where there is another operator that modifies resources or when admission controllers changes resources.

Which issue(s) does this PR fix or relate to

https://issues.redhat.com/browse/RHIDP-4189

PR acceptance criteria

Tests
Documentation
If the bundle manifests have been updated, make sure to review the rhdh-operator.csv.yaml file accordingly

How to test changes / Special notes to the reviewer

Signed-off-by: gazarenkov <[email protected]>

gazarenkov

Like this approach LGTM, let's observe and see if we can even more optimize it.

rm3l

Thanks for this!
I tested it, and while it works great for new installs of the operator, I came across an issue when testing the upgrade path, from 1.2.x to this branch, with an existing operator-backed deployment.

Steps to reproduce the upgrade path (which can also be reproduced by running make test-e2e-upgrade [IMG=<image-build-of-the-operator-from-this-branch>]):

Switch to the 1.2.x branch and run the operator with make install run
In a separate terminal, create a CR. I used the simple https://github.com/redhat-developer/rhdh-operator/blob/1.2.x/examples/bs1.yaml
Wait for the application pods to be running and ready:

oc describe backstage bs1

$ oc describe backstages.rhdh.redhat.com/bs1 
Name:         bs1
Namespace:    my-ns
Labels:       <none>
Annotations:  <none>
API Version:  rhdh.redhat.com/v1alpha1
Kind:         Backstage
Metadata:
  Creation Timestamp:  2024-08-21T09:33:38Z
  Generation:          1
  Resource Version:    70209
  UID:                 9b2c3e73-094e-4515-adbd-05bef70b3f24
Status:
  Conditions:
    Last Transition Time:  2024-08-21T09:33:41Z
    Message:               
    Reason:                Deployed
    Status:                True
    Type:                  Deployed
Events:                    <none>

Stop the operator running from step 1
Switch to this PR branch and run the operator with make install run

At this point, it should reconcile successfully, but it looks like there is a reconciliation error:

$ oc describe backstages.rhdh.redhat.com/bs1                                   
Name:         bs1
Namespace:    my-ns
Labels:       <none>
Annotations:  <none>
API Version:  rhdh.redhat.com/v1alpha2
Kind:         Backstage
Metadata:
  Creation Timestamp:  2024-08-21T09:14:08Z
  Generation:          1
  Resource Version:    65153
  UID:                 eb0a158e-01a6-4b9e-84d3-8fc61fc43072
Status:
  Conditions:
    Last Transition Time:  2024-08-21T09:19:41Z
    Message:               failed to apply backstage objects failed to create secret: Apply failed with 1 conflict: conflict with "manager" using v1: .metadata.ownerReferences[uid="eb0a158e-01a6-4b9e-84d3-8fc61fc43072"]
    Reason:                DeployFailed
    Status:                False
    Type:                  Deployed
Events:                    <none>

I found a lot of these errors in the operator logs:

failed to apply backstage objects failed to create secret: Apply failed with 1 conflict: conflict with \"manager\" using v1: .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]

Full logs

☸ my-ns/api-ci-ln-5nxf3cb-72292-origin-ci-int-gce-dev-rhcloud-com:6443/kube:admin (my-ns) in ~/w/p/b/j/operator on  use-server-side-apply [$!] via 🐹 v1.21.12
$ make install run                                                                                                                                                         11:19:28 [160/10442]
test -s /home/asoro/work/projects/backstage/janus-idp/operator/bin/controller-gen || GOBIN=/home/asoro/work/projects/backstage/janus-idp/operator/bin go install sigs.k8s.io/controller-tools/c
md/[email protected]                                                                                                                                                                      
/home/asoro/work/projects/backstage/janus-idp/operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases                    
test -s /home/asoro/work/projects/backstage/janus-idp/operator/bin/kustomize || { curl -Ss "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" | bas
h -s -- 5.4.2 /home/asoro/work/projects/backstage/janus-idp/operator/bin; }                                                                                                                    
/home/asoro/work/projects/backstage/janus-idp/operator/bin/kustomize build config/crd | kubectl apply -f -                                                                                     
customresourcedefinition.apiextensions.k8s.io/backstages.rhdh.redhat.com configured                                                                                                            
/home/asoro/work/projects/backstage/janus-idp/operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."                                                            
test -s /home/asoro/work/projects/backstage/janus-idp/operator/bin/goimports || GOBIN=/home/asoro/work/projects/backstage/janus-idp/operator/bin go install golang.org/x/tools/cmd/goimports@v0
.16.1                                                                                                                                                                                          
find . -not -path '*/\.*' -name '*.go' -exec /home/asoro/work/projects/backstage/janus-idp/operator/bin/goimports -w {} \;                                                                     
go vet ./...                                                                                                                                                                                   
go build -o bin/manager main.go                                                                                                                                                                
cd /home/asoro/work/projects/backstage/janus-idp/operator/bin && mkdir -p default-config && cp ../config/manager/default-config/* default-config && ./manager
2024-08-21T11:19:40+02:00       INFO    setup   starting manager with parameters:       {"own-runtime": true, "env.LOCALBIN": "/home/asoro/work/projects/backstage/janus-idp/operator/bin", "is
OpenShift": true}
2024-08-21T11:19:40+02:00       INFO    controller-runtime.metrics      Starting metrics server
2024-08-21T11:19:40+02:00       INFO    starting server {"kind": "health probe", "addr": "[::]:8081"}
2024-08-21T11:19:40+02:00       INFO    controller-runtime.metrics      Serving metrics server  {"bindAddress": ":8080", "secure": false}
2024-08-21T11:19:40+02:00       INFO    Starting EventSource    {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "source": "kind source: *v1alp
ha2.Backstage"}
2024-08-21T11:19:40+02:00       INFO    Starting EventSource    {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "source": "kind source: *v1.Pa
rtialObjectMetadata"}
2024-08-21T11:19:40+02:00       INFO    Starting EventSource    {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "source": "kind source: *v1.Pa
rtialObjectMetadata"}
2024-08-21T11:19:40+02:00       INFO    Starting Controller     {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage"}
2024-08-21T11:19:40+02:00       INFO    Starting workers        {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "worker count": 1}
2024-08-21T11:19:41+02:00       DEBUG   failed to patch object => trying to delete it (and losing any custom labels/annotations on it) so it can be recreated upon next reconciliation...     {
"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","namespace":"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcil
eID": "e2fbadf5-279c-4024-bb1b-57c8c631e709", "*v1.ConfigMap": "backstage-appconfig-bs1", "cause": "failed to patch object *v1.ConfigMap: Apply failed with 2 conflicts: conflicts with \"manag
er\" using v1:\n- .data.default.app-config.yaml\n- .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]"}
2024-08-21T11:19:41+02:00       DEBUG   deleted object. If you had set any custom labels/annotations on it manually, you will need to add them again    {"controller": "backstage", "controller
Group": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","namespace":"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "e2fbadf5-279c-4024-bb1b-57c8c63
1e709", "*v1.ConfigMap": "backstage-appconfig-bs1"}
2024-08-21T11:19:41+02:00       ERROR   Reconciler error        {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage11:19:42 [125/10442]
mespace":"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "e2fbadf5-279c-4024-bb1b-57c8c631e709", "error": "failed to apply backstage objects failed to create secret: Apply faile
d with 1 conflict: conflict with \"manager\" using v1: .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]"}                                                               
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler                                                                                                          
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:329
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
2024-08-21T11:19:42+02:00       DEBUG   create object   {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","namespace"
:"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "b28cbcbf-8ae1-4072-9e2a-d235fed2c014", "*v1.ConfigMap": "backstage-appconfig-bs1"}
2024-08-21T11:19:42+02:00       ERROR   Reconciler error        {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","na
mespace":"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "b28cbcbf-8ae1-4072-9e2a-d235fed2c014", "error": "failed to apply backstage objects failed to create secret: Apply faile
d with 1 conflict: conflict with \"manager\" using v1: .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:329
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
2024-08-21T11:19:42+02:00       DEBUG   patch object    {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","namespace"
:"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "403a163e-3e39-48cb-8800-ccbcb9406d72", "*v1.ConfigMap": "backstage-appconfig-bs1"}
2024-08-21T11:19:42+02:00       ERROR   Reconciler error        {"controller": "backstage", "controllerGroup": "rhdh.redhat.com", "controllerKind": "Backstage", "Backstage": {"name":"bs1","na
mespace":"my-ns"}, "namespace": "my-ns", "name": "bs1", "reconcileID": "403a163e-3e39-48cb-8800-ccbcb9406d72", "error": "failed to apply backstage objects failed to create secret: Apply faile
d with 1 conflict: conflict with \"manager\" using v1: .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:329
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /home/asoro/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227

Could you look into this?

/hold

controllers/backstage_controller.go

kadel · 2024-08-21T12:44:54Z

Good catch. I'll look into this.
But the priority of this just becomes lower. This was originally a blocker for implementing PVC support for 1.3, but we won't be adding this to next release, so we can figure out this later.

Signed-off-by: gazarenkov <[email protected]>

…erver-side-apply

github-actions · 2024-09-18T08:03:18Z

PR images are available:

Signed-off-by: gazarenkov <[email protected]>

github-actions · 2024-09-18T12:11:14Z

PR images are available:

Signed-off-by: gazarenkov <[email protected]> # Conflicts: # controllers/backstage_controller.go

github-actions · 2024-09-25T07:12:07Z

PR images are available:

Signed-off-by: gazarenkov <[email protected]>

github-actions · 2024-09-25T07:44:34Z

PR images are available:

gazarenkov · 2024-09-25T07:54:51Z

Thanks for this! I tested it, and while it works great for new installs of the operator, I came across an issue when testing the upgrade path, from 1.2.x to this branch, with an existing operator-backed deployment.

Steps to reproduce the upgrade path (which can also be reproduced by running make test-e2e-upgrade [IMG=<image-build-of-the-operator-from-this-branch>]):

Switch to the 1.2.x branch and run the operator with make install run

In a separate terminal, create a CR. I used the simple https://github.com/redhat-developer/rhdh-operator/blob/1.2.x/examples/bs1.yaml

Wait for the application pods to be running and ready:

oc describe backstage bs1
4. Stop the operator running from step 1
5. Switch to this PR branch and run the operator with make install run

At this point, it should reconcile successfully, but it looks like there is a reconciliation error:
$ oc describe backstages.rhdh.redhat.com/bs1                                   
Name:         bs1
Namespace:    my-ns
Labels:       <none>
Annotations:  <none>
API Version:  rhdh.redhat.com/v1alpha2
Kind:         Backstage
Metadata:
  Creation Timestamp:  2024-08-21T09:14:08Z
  Generation:          1
  Resource Version:    65153
  UID:                 eb0a158e-01a6-4b9e-84d3-8fc61fc43072
Status:
  Conditions:
    Last Transition Time:  2024-08-21T09:19:41Z
    Message:               failed to apply backstage objects failed to create secret: Apply failed with 1 conflict: conflict with "manager" using v1: .metadata.ownerReferences[uid="eb0a158e-01a6-4b9e-84d3-8fc61fc43072"]
    Reason:                DeployFailed
    Status:                False
    Type:                  Deployed
Events:                    <none>
I found a lot of these errors in the operator logs:
failed to apply backstage objects failed to create secret: Apply failed with 1 conflict: conflict with \"manager\" using v1: .metadata.ownerReferences[uid=\"eb0a158e-01a6-4b9e-84d3-8fc61fc43072\"]
Full logs
Could you look into this?

/hold

this is presumably fixed

controllers/backstage_controller.go

github-actions · 2024-09-26T15:56:28Z

PR images are available:

rm3l · 2024-09-26T16:01:09Z

/lgtm

LGTM since 2 people already worked on this PR.

openshift-ci · 2024-09-26T16:01:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rm3l]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fix: use server-side-apply for all operations

df2cbad

openshift-ci bot requested review from coreydaley and nickboldt August 15, 2024 12:13

kadel temporarily deployed to internal August 15, 2024 12:13 — with GitHub Actions Inactive

kadel changed the title ~~fix: use server-side-apply for all operations~~ [WIP] fix: use server-side-apply for all operations Aug 15, 2024

openshift-ci bot added the do-not-merge/work-in-progress label Aug 15, 2024

kadel marked this pull request as draft August 15, 2024 12:32

kadel changed the title ~~[WIP] fix: use server-side-apply for all operations~~ fix: use server-side-apply for all operations Aug 15, 2024

generated

c5ad916

Signed-off-by: gazarenkov <[email protected]>

gazarenkov temporarily deployed to internal August 15, 2024 21:12 — with GitHub Actions Inactive

comment out unnecessary hack for openshift?

586f72c

kadel temporarily deployed to internal August 19, 2024 10:17 — with GitHub Actions Inactive

kadel marked this pull request as ready for review August 20, 2024 11:03

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 20, 2024

kadel temporarily deployed to internal August 20, 2024 11:03 — with GitHub Actions Inactive

openshift-ci bot requested a review from rm3l August 20, 2024 11:03

gazarenkov self-requested a review August 20, 2024 12:52

gazarenkov previously approved these changes Aug 20, 2024

View reviewed changes

openshift-ci bot added the approved label Aug 20, 2024

kadel temporarily deployed to internal August 21, 2024 08:38 — with GitHub Actions Inactive

rm3l reviewed Aug 21, 2024

View reviewed changes

controllers/backstage_controller.go Outdated Show resolved Hide resolved

openshift-ci bot added the do-not-merge/hold label Aug 21, 2024

gazarenkov self-requested a review September 17, 2024 13:54

gazarenkov added 3 commits September 17, 2024 16:55

Merge remote-tracking branch 'upstream/main' into use-server-side-apply

4573fe4

create for Secrets, force conflicts

91371eb

Signed-off-by: gazarenkov <[email protected]>

Merge remote-tracking branch 'kadel/use-server-side-apply' into use-s…

3e8238d

…erver-side-apply

gazarenkov temporarily deployed to internal September 18, 2024 07:58 — with GitHub Actions Inactive

log

a160984

Signed-off-by: gazarenkov <[email protected]>

gazarenkov temporarily deployed to internal September 18, 2024 12:06 — with GitHub Actions Inactive

Merge remote-tracking branch 'upstream/main' into use-server-side-apply

0e6ca37

Signed-off-by: gazarenkov <[email protected]> # Conflicts: # controllers/backstage_controller.go

gazarenkov temporarily deployed to internal September 25, 2024 07:06 — with GitHub Actions Inactive

fix upgrade glitch

05dbb17

Signed-off-by: gazarenkov <[email protected]>

gazarenkov temporarily deployed to internal September 25, 2024 07:39 — with GitHub Actions Inactive

gazarenkov removed the do-not-merge/hold label Sep 25, 2024

gazarenkov requested a review from rm3l September 25, 2024 07:55

rm3l reviewed Sep 26, 2024

View reviewed changes

controllers/backstage_controller.go Outdated Show resolved Hide resolved

Update controllers/backstage_controller.go

9d0c6ec

rm3l temporarily deployed to internal September 26, 2024 15:51 — with GitHub Actions Inactive

rm3l approved these changes Sep 26, 2024

View reviewed changes

openshift-ci bot added the approved label Sep 26, 2024

openshift-ci bot assigned rm3l Sep 26, 2024

openshift-ci bot added the lgtm label Sep 26, 2024

openshift-merge-bot bot merged commit 8bd5473 into redhat-developer:main Sep 26, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use server-side-apply for all operations #80

fix: use server-side-apply for all operations #80

kadel commented Aug 15, 2024 •

edited by gazarenkov

Loading

gazarenkov left a comment

rm3l left a comment •

edited

Loading

kadel commented Aug 21, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 25, 2024

github-actions bot commented Sep 25, 2024

gazarenkov commented Sep 25, 2024

github-actions bot commented Sep 26, 2024

rm3l commented Sep 26, 2024 •

edited

Loading

openshift-ci bot commented Sep 26, 2024

fix: use server-side-apply for all operations #80

fix: use server-side-apply for all operations #80

Conversation

kadel commented Aug 15, 2024 • edited by gazarenkov Loading

Description

Which issue(s) does this PR fix or relate to

PR acceptance criteria

How to test changes / Special notes to the reviewer

gazarenkov left a comment

Choose a reason for hiding this comment

rm3l left a comment • edited Loading

Choose a reason for hiding this comment

kadel commented Aug 21, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 25, 2024

github-actions bot commented Sep 25, 2024

gazarenkov commented Sep 25, 2024

github-actions bot commented Sep 26, 2024

rm3l commented Sep 26, 2024 • edited Loading

openshift-ci bot commented Sep 26, 2024

kadel commented Aug 15, 2024 •

edited by gazarenkov

Loading

rm3l left a comment •

edited

Loading

rm3l commented Sep 26, 2024 •

edited

Loading