open-policy-agent / kube-mgmt

Sidecar for managing OPA instances in Kubernetes.
Apache License 2.0
235 stars 105 forks source link

Add startup probe to kube-mgmt container #210

Closed eshepelyuk closed 1 year ago

eshepelyuk commented 1 year ago

Kube-mgmt container should not start before OPA container. I.e. kube-mgmt startup probe should query OPA API for availability.

By default the health API should be queried

https://www.openpolicyagent.org/docs/latest/rest-api/#health-api

The startup probe API URL should be overridable.

This ensures that kube mgmt will be working with live OPA container, thus avoiding issues described in #206

https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-startup-probes

Relates #189 Relates #206

saranyareddy24 commented 1 year ago

Hi @eshepelyuk, Can I contribute in solving this issue.

eshepelyuk commented 1 year ago

Hi @eshepelyuk, Can I contribute in solving this issue.

you're very welcome

saranyareddy24 commented 1 year ago

Hi @eshepelyuk, In the description, it was mentioned kube-mgmt should query for OPA API. Could you please mention what is the API which kube-mgmt has to probe for.

eshepelyuk commented 1 year ago

Hi @eshepelyuk, In the description, it was mentioned kube-mgmt should query for OPA API. Could you please mention what is the API which kube-mgmt has to probe for.

Hello

Updated the description.

saranyareddy24 commented 1 year ago

Tested with the below configuration, pod is coming up fine. Let me know if this is okay, so that I can raise PR.

      startupProbe:
          failureThreshold: 5
          httpGet:
            path: /health
            port: 8443
            scheme: HTTPS
          initialDelaySeconds: 20
          periodSeconds: 5
          successThreshold: 1
          timeoutSeconds: 10
eshepelyuk commented 1 year ago

Tested with the below configuration, pod is coming up fine. Let me know if this is okay, so that I can raise PR.

      startupProbe:
          failureThreshold: 1
          httpGet:
            path: /health
            port: 8443
            scheme: HTTPS
          initialDelaySeconds: 5
          periodSeconds: 5
          successThreshold: 1
          timeoutSeconds: 1

I believe that is wrong. Assuming this check is for kube-mgmt container, this check is going to check kube-mgmt container itself. While rhe check should reach OPA container.

eshepelyuk commented 1 year ago

But anyways, plz provide a PR so I can see he whole scope of changes.

saranyareddy24 commented 1 year ago

Correct me if I am wrong.

I have a pod where OPA and kube-mgmt are running together. I logged into kube-mgmt and executed a POST request to 127.0.0.1:8443.

I believe the response is coming from OPA container.

kc exec -it opa-66677d6767-2kv2q -n fed-opa -c kube-mgmt sh
/ # wget --server-response https://127.0.0.1:8443 --post-data {} --no-check-certificate
Connecting to 127.0.0.1:8443 (127.0.0.1:8443)
  HTTP/1.1 200 OK
  Content-Type: application/json
  Date: Sat, 10 Jun 2023 10:42:52 GMT
  Content-Length: 95
  Connection: close
saving to 'index.html'
index.html           100% |***************************************************************************|    95  0:00:00 ETA
'index.html' saved

/ # cat index.html
{"apiVersion":"admission.k8s.io/v1beta1","kind":"AdmissionReview","response":{"allowed":true}}
/ #

Similarly when I try to check Health API

/ # wget --server-response https://127.0.0.1:8443/health --no-check-certificate
Connecting to 127.0.0.1:8443 (127.0.0.1:8443)
  HTTP/1.1 200 OK
  Content-Type: application/json
  Date: Sat, 10 Jun 2023 10:47:22 GMT
  Content-Length: 3
  Connection: close

saving to 'health'
health               100% |***************************************************************************|     3  0:00:00 ETA
'health' saved

/ # cat health
{}

As per the documentation from OPA they expect {} if the container is healthy.

eshepelyuk commented 1 year ago

Hello @saranyareddy24

This last example is correct.

saranyareddy24 commented 1 year ago

In that case, the configuration which I mentioned, should be fine right?

eshepelyuk commented 1 year ago

In that case, the configuration which I mentioned, should be fine right?

yeah looks like I've missed the port configuration in your 1st post.

saranyareddy24 commented 1 year ago

Okay. Thanks. I will raise PR