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

DOC: Liveness/Readiness Probe's default values are inconsistent with the documentation #289

Closed
hoyhbx opened this issue Jun 17, 2022 · 1 comment · Fixed by #302
Closed
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@hoyhbx
Copy link
Contributor

hoyhbx commented Jun 17, 2022

We found that Liveness/Readiness Probe's default values are inconsistent as what is stated in documentation and Custom Resource Definition.

What version of redis operator are you using?

redis-operator version: We are using redis-operator built from the HEAD

Does this issue reproduce with the latest release?

Yes, it reproduces with quay.io/opstree/redis-operator:v0.10.0

What operating system and processor architecture are you using (kubectl version)?

kubectl version Output
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.1", GitCommit:"3ddd0f45aa91e2f30c70734b175631bec5b5825a", GitTreeState:"clean", BuildDate:"2022-05-24T12:26:19Z", GoVersion:"go1.18.2", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.9", GitCommit:"6df4433e288edc9c40c2e344eb336f63fad45cd2", GitTreeState:"clean", BuildDate:"2022-05-19T19:53:08Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}

What did you do?

I tried to create a very basic redis cluster. To do this, I applied the below YAML file. Note that the livenessProbe and readinessProbe are left unspecified.

apiVersion: redis.redis.opstreelabs.in/v1beta1
kind: RedisCluster
metadata:
  name: test-cluster
spec:
  clusterSize: 3
  kubernetesConfig:
    image: quay.io/opstree/redis:v6.2.5
    imagePullPolicy: IfNotPresent
    resources:
      limits:
        cpu: 101m
        memory: 128Mi
      requests:
        cpu: 101m
        memory: 128Mi
  storage:
    volumeClaimTemplate:
      spec:
        accessModes:
        - ReadWriteOnce
        resources:
          requests:
            storage: 1Gi

What did you expect to see?

By querying YAML spec of pods, I expect to see that their livenessProbe and readinessProbe are set to the claimed default value as specified in the CRD. I expect to see the following default values.

  • failureThreshold: 3
  • periodSeconds: 10
  • successThreshold: 1
  • timeoutSeconds: 1

What did you see instead?

The pods have the following default value for probes:

  • failureThreshold: 5
  • periodSeconds: 15
  • successThreshold: 1
  • timeoutSeconds: 5
  • initialDelaySeconds: 15

Possible root cause and Suggested Fix

The default values for livenessProbe and readinessProbe are assigned in generateContainerDef from which the default values are from the return value of getProbeInfo. However, getProbeInfo's return value is inconsistent with the CRD.

If it is intended behavior that the operator override Kubernetes' default probing configuration, we think it is important that the operator check for min and max values for a specific field, because otherwise kubernetes would override the operator's default value.

For example, the operator's default value for periodSeconds is 15. If we submit periodSeconds: 0, since the lower limit is 1, the value are expected be overridden by the default value, which is 15. However, since operator does not realize that 0 is smaller than the lower limit and submits the value to kubernetes, the value will be defaulted to 10, which is the kubernetes-defined default value.

In addition, we suggest that CRD be modified to reflect the true default values for the probing fields.

@hoyhbx hoyhbx added the bug Something isn't working label Jun 17, 2022
@iamabhishek-dubey iamabhishek-dubey added good first issue Good for newcomers labels Jul 11, 2022
@iamabhishek-dubey
Copy link
Member

Relates to #297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants