For Ansible/Helm-based operators, add Liveness and Readiness probe #4326

camilamacedo86 · 2020-12-16T18:19:21Z

Description of the change:

Add the probes for Helm/Ansible by default as it was done for Golang go/v3 in upstream.
For Ansible/Helm-based operators, added new flag --health-probe-bind-address to allow customize the probe port used.
For Ansible-based operators, deprecated the ping endpoint

Motivation for the change:

Health Check (Liveness and Readiness probe for Operator) #1234

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
Add or update relevant sections of the docs website in website/content/en/docs

internal/cmd/ansible-operator/run/cmd.go

estroz · 2020-12-16T18:43:21Z

One thing to be aware of (not blocking), and probably encoded in the changelog fragment's migration guide: these probes now are hard-coded to healthz.Ping and can't be configured by helm/ansible artifacts.

asmacdo

/lgtm

changelog/fragments/liveness_readiness_probe_for_operator.yaml

asmacdo · 2020-12-16T19:12:06Z

internal/cmd/ansible-operator/run/cmd.go

@@ -183,6 +179,7 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 		}, w.Blacklist)
 	}

+	// todo: remove when a upper version be bumped


it might be a good idea to file an issue for this and put a "backwards incompatible" label, or put this in the 2.0 milestone. Otherwise, we will probably forget this when we bump to 2.0.0

Done : https://github.com/operator-framework/operator-sdk/issues/4330

openshift-ci-robot · 2020-12-16T19:19:49Z

New changes are detected. LGTM label has been removed.

changelog/fragments/liveness_readiness_probe_for_operator.yaml

internal/ansible/flags/flag.go

internal/helm/flags/flag.go

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go

internal/plugins/helm/v1/scaffolds/internal/templates/config/manager/manager.go

changelog/fragments/liveness_readiness_probe_for_operator.yaml

jmrodri

I agree with @estroz that the port should remain the same, and if people want to change it they can specify 8081. I know this means that go and ansible/helm will diverge, but I think that's okay.

changelog/fragments/liveness_readiness_probe_for_operator.yaml

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

all suggestions are addressed so it shows fine for we move on.

…perator-framework#4326) **Description of the change:** - Add the probes for Helm/Ansible by default as it was done for Golang go/v3 in upstream. - For Ansible/Helm-based operators, added new flag `--health-probe-bind-address` to allow customize the probe port used. - For Ansible-based operators, deprecated the ping endpoint **Motivation for the change:** - operator-framework#1234 Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>

…perator-framework#4326) **Description of the change:** - Add the probes for Helm/Ansible by default as it was done for Golang go/v3 in upstream. - For Ansible/Helm-based operators, added new flag `--health-probe-bind-address` to allow customize the probe port used. - For Ansible-based operators, deprecated the ping endpoint **Motivation for the change:** - operator-framework#1234 Signed-off-by: rearl <rearl@secureworks.com>

flozzone

The Upgrade SDK version guide for version 1.4.2 contains a wrong path to the manager resource in the For Helm-based operators, add Liveness and Readiness probe and For Ansible-based operators, add Liveness and Readiness probe section.

flozzone · 2021-02-16T10:03:46Z

changelog/fragments/liveness_readiness_probe_for_operator.yaml

+
+        You can update your pre-existing project to use them. For that update the Dockerfile to use the latest
+        release base image, then add the following to the `manager` container in
+        `config/default/manager/manager.yaml`:


The path to the manager resource seems to be wrong. Should be config/manager/manager.yaml instead.

flozzone

I always compare (diff -r) my repository with the one freshly generated by operator-sdk init to see which things I changed or have been introduced by a new version. While doing so, I saw that the manager resource includes the changes mentioned in version 1.4.0 of the 'Upgrade SDK version guide. But while the guide says to use /readyzendpoint for the readinessProbe and/healthz` endpoint for the livenessProbe, the generated project uses the endpoints the other way around:

config/manager/manager.yaml:

          livenessProbe:
            httpGet:
              path: /readyz
              port: 6789
            initialDelaySeconds: 15
            periodSeconds: 20
          readinessProbe:
            httpGet:
              path: /healthz
              port: 6789
            initialDelaySeconds: 5
            periodSeconds: 10

I guess the upgrade guide shows it correctly but is wrong for generated projects.

$ operator-sdk version
operator-sdk version: "v1.4.2", commit: "4b083393be65589358b3e0416573df04f4ae8d9b", kubernetes version: "1.19.4", go version: "go1.15.5", GOOS: "linux", GOARCH: "amd64"

Should I open an issue for this?

flozzone · 2021-02-16T10:32:10Z

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go

@@ -78,5 +78,17 @@ spec:
            - name: ANSIBLE_GATHERING
              value: explicit
          image: {{ .Image }}
+          livenessProbe:
+            httpGet:
+              path: /readyz


livenessProbe using /readyz endpoint

flozzone · 2021-02-16T10:32:42Z

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go

+            periodSeconds: 20
+          readinessProbe:
+            httpGet:
+              path: /healthz


readinessProbe using /healthz endpoint

flozzone · 2021-02-16T10:34:29Z

...ta/ansible/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml

+                livenessProbe:
+                  httpGet:
+                    path: /readyz
+                    port: 6789
+                  initialDelaySeconds: 15
+                  periodSeconds: 20
                name: manager
+                readinessProbe:
+                  httpGet:
+                    path: /healthz
+                    port: 6789
+                  initialDelaySeconds: 5
+                  periodSeconds: 10


livenessProbe using /readyz endpoint and readinessProbe using /healthz endpoint. Should be the other way around I guess.

flozzone · 2021-02-16T10:34:48Z

testdata/ansible/memcached-operator/config/manager/manager.yaml

+          livenessProbe:
+            httpGet:
+              path: /readyz
+              port: 6789
+            initialDelaySeconds: 15
+            periodSeconds: 20
+          readinessProbe:
+            httpGet:
+              path: /healthz
+              port: 6789
+            initialDelaySeconds: 5
+            periodSeconds: 10


livenessProbe using /readyz endpoint and readinessProbe using /healthz endpoint. Should be the other way around I guess.

flozzone · 2021-02-16T10:35:08Z

testdata/helm/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml

+                livenessProbe:
+                  httpGet:
+                    path: /readyz
+                    port: 8081
+                  initialDelaySeconds: 15
+                  periodSeconds: 20
                name: manager
+                readinessProbe:
+                  httpGet:
+                    path: /healthz
+                    port: 8081
+                  initialDelaySeconds: 5
+                  periodSeconds: 10


livenessProbe using /readyz endpoint and readinessProbe using /healthz endpoint. Should be the other way around I guess.

flozzone · 2021-02-16T10:35:20Z

testdata/helm/memcached-operator/config/manager/manager.yaml

+        livenessProbe:
+          httpGet:
+            path: /readyz
+            port: 8081
+          initialDelaySeconds: 15
+          periodSeconds: 20
+        readinessProbe:
+          httpGet:
+            path: /healthz
+            port: 8081
+          initialDelaySeconds: 5
+          periodSeconds: 10


livenessProbe using /readyz endpoint and readinessProbe using /healthz endpoint. Should be the other way around I guess.

camilamacedo86 · 2021-02-16T13:27:28Z

HI @flozzone,

Thank you for your contribution? WDYT about todo a pr against the branches and master to update the info.?

Related to operator-sdk v1.4.0 upgrade: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.4.0/ operator-framework/operator-sdk#4326 Signed-off-by: Wayne Sun <gsun@redhat.com>

openshift-ci-robot requested review from fabianvf and rashmigottipati December 16, 2020 18:19

camilamacedo86 mentioned this pull request Dec 16, 2020

Health Check (Liveness and Readiness probe for Operator) #1234

Closed

camilamacedo86 requested review from asmacdo, estroz and jmrodri December 16, 2020 18:21

camilamacedo86 force-pushed the issue-1234 branch from b7678e1 to 068862c Compare December 16, 2020 18:29

estroz reviewed Dec 16, 2020

View reviewed changes

internal/cmd/ansible-operator/run/cmd.go Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/cmd/ansible-operator/run/cmd.go Outdated Show resolved Hide resolved

estroz previously requested changes Dec 16, 2020

View reviewed changes

internal/cmd/ansible-operator/run/cmd.go Show resolved Hide resolved

asmacdo approved these changes Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

openshift-ci-robot assigned asmacdo Dec 16, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2020

asmacdo reviewed Dec 16, 2020

View reviewed changes

camilamacedo86 force-pushed the issue-1234 branch from 26b2ac6 to 1f4ace5 Compare December 16, 2020 19:19

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2020

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/ansible/flags/flag.go Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/helm/flags/flag.go Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/plugins/helm/v1/scaffolds/internal/templates/config/manager/manager.go Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

internal/plugins/helm/v1/scaffolds/internal/templates/config/manager/manager.go Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the issue-1234 branch from 1f4ace5 to 8cca5f6 Compare December 16, 2020 20:07

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

jmrodri requested changes Dec 16, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

camilamacedo86 requested review from estroz and jmrodri December 17, 2020 09:46

camilamacedo86 mentioned this pull request Oct 5, 2023

Ansible using the same default port that is used for Go and Helm operator-framework/ansible-operator-plugins#23

Open

For Ansible/Helm-based operators, add Liveness and Readiness probe

5a85561

camilamacedo86 force-pushed the issue-1234 branch from 0c849cd to 5a85561 Compare December 17, 2020 10:15

estroz reviewed Dec 17, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

estroz reviewed Dec 17, 2020

View reviewed changes

changelog/fragments/liveness_readiness_probe_for_operator.yaml Outdated Show resolved Hide resolved

camilamacedo86 and others added 2 commits December 17, 2020 19:12

Update changelog/fragments/liveness_readiness_probe_for_operator.yaml

61b639d

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

Update changelog/fragments/liveness_readiness_probe_for_operator.yaml

e34525a

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

camilamacedo86 requested a review from estroz December 17, 2020 19:12

jmrodri approved these changes Dec 18, 2020

View reviewed changes

camilamacedo86 merged commit ea43495 into operator-framework:master Dec 18, 2020

camilamacedo86 deleted the issue-1234 branch December 18, 2020 16:34

flozzone reviewed Feb 16, 2021

View reviewed changes

flozzone mentioned this pull request Feb 16, 2021

Swapped livenessProbe and readinessProbe endpoints in 1.4.2 #4541

Closed

kd7lxl mentioned this pull request Jul 25, 2023

add probes to chart infobloxopen/konk#436

Merged

ramperher mentioned this pull request Jan 9, 2024

Support startup probes #6659

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For Ansible/Helm-based operators, add Liveness and Readiness probe #4326

For Ansible/Helm-based operators, add Liveness and Readiness probe #4326

camilamacedo86 commented Dec 16, 2020 •

edited

Loading

estroz commented Dec 16, 2020

asmacdo left a comment

asmacdo Dec 16, 2020

camilamacedo86 Dec 17, 2020

openshift-ci-robot commented Dec 16, 2020

jmrodri left a comment

flozzone left a comment •

edited

Loading

flozzone Feb 16, 2021

flozzone left a comment

flozzone Feb 16, 2021

flozzone Feb 16, 2021

flozzone Feb 16, 2021

flozzone Feb 16, 2021

flozzone Feb 16, 2021

flozzone Feb 16, 2021

camilamacedo86 commented Feb 16, 2021 •

edited

Loading

For Ansible/Helm-based operators, add Liveness and Readiness probe #4326

For Ansible/Helm-based operators, add Liveness and Readiness probe #4326

Conversation

camilamacedo86 commented Dec 16, 2020 • edited Loading

estroz commented Dec 16, 2020

asmacdo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Dec 16, 2020

jmrodri left a comment

Choose a reason for hiding this comment

flozzone left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flozzone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 commented Feb 16, 2021 • edited Loading

camilamacedo86 commented Dec 16, 2020 •

edited

Loading

flozzone left a comment •

edited

Loading

camilamacedo86 commented Feb 16, 2021 •

edited

Loading