redis-operator icon indicating copy to clipboard operation
redis-operator copied to clipboard

CRD must not contain spec.conversion webhook link when the webhook is disabled

Open davidpechcz opened this issue 1 year ago • 4 comments

What version of redis operator are you using? HELM chart: 0.15.9

operator logs:

{"level":"info","ts":1701246758.8115792,"logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":":8080"}
{"level":"info","ts":1701246758.8121943,"logger":"setup","msg":"starting manager"}
{"level":"info","ts":1701246758.8126636,"msg":"Starting server","kind":"health probe","addr":"[::]:8081"}
I1129 08:32:38.812771 1 leaderelection.go:248] attempting to acquire leader lease redis-operator/6cab913b.redis.opstreelabs.in...
{"level":"info","ts":1701246758.8132775,"msg":"Starting server","path":"/metrics","kind":"metrics","addr":"[::]:8080"}
I1129 08:33:00.727883 1 leaderelection.go:258] successfully acquired lease redis-operator/6cab913b.redis.opstreelabs.in
{"level":"info","ts":1701246780.728259,"logger":"controller.redis","msg":"Starting EventSource","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"Redis","source":"kind source: *v1beta2.Redis"}
{"level":"info","ts":1701246780.7285497,"logger":"controller.redis","msg":"Starting Controller","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"Redis"}
{"level":"info","ts":1701246780.72843,"logger":"controller.rediscluster","msg":"Starting EventSource","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisCluster","source":"kind source: *v1beta2.RedisCluster"}
{"level":"info","ts":1701246780.7287045,"logger":"controller.rediscluster","msg":"Starting Controller","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisCluster"}
{"level":"info","ts":1701246780.7283218,"logger":"controller.redisreplication","msg":"Starting EventSource","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisReplication","source":"kind source: *v1beta2.RedisReplication"}
{"level":"info","ts":1701246780.7288792,"logger":"controller.redisreplication","msg":"Starting Controller","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisReplication"}
{"level":"info","ts":1701246780.7284963,"logger":"controller.redissentinel","msg":"Starting EventSource","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisSentinel","source":"kind source: *v1beta2.RedisSentinel"}
{"level":"info","ts":1701246780.7290123,"logger":"controller.redissentinel","msg":"Starting Controller","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisSentinel"}
{"level":"info","ts":1701246780.830468,"logger":"controller.redissentinel","msg":"Starting workers","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisSentinel","worker count":1}
{"level":"info","ts":1701246780.8304439,"logger":"controller.redis","msg":"Starting workers","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"Redis","worker count":1}
{"level":"info","ts":1701246780.830484,"logger":"controller.rediscluster","msg":"Starting workers","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisCluster","worker count":1}
{"level":"info","ts":1701246780.8304384,"logger":"controller.redisreplication","msg":"Starting workers","reconciler group":"redis.redis.opstreelabs.in","reconciler kind":"RedisReplication","worker count":1}
{"level":"info","ts":1701246780.8306277,"logger":"controllers.Redis","msg":"Reconciling opstree redis controller","Request.Namespace":"makro-master","Request.Name":"redis-cache"}
{"level":"info","ts":1701246780.8883927,"logger":"controllers.Redis","msg":"Will reconcile redis operator in again 10 seconds","Request.Namespace":"makro-master","Request.Name":"redis-cache"}
{"level":"info","ts":1701246780.8886113,"logger":"controllers.Redis","msg":"Reconciling opstree redis controller","Request.Namespace":"makro-master","Request.Name":"redis-prometheus"}
{"level":"info","ts":1701246780.9711776,"logger":"controllers.Redis","msg":"Will reconcile redis operator in again 10 seconds","Request.Namespace":"makro-master","Request.Name":"redis-prometheus"}
{"level":"info","ts":1701246780.9714046,"logger":"controllers.Redis","msg":"Reconciling opstree redis controller","Request.Namespace":"makro-master","Request.Name":"redis-session"}
{"level":"info","ts":1701246781.062171,"logger":"controllers.Redis","msg":"Will reconcile redis operator in again 10 seconds","Request.Namespace":"makro-master","Request.Name":"redis-session"}
{"level":"info","ts":1701246781.062256,"logger":"controllers.Redis","msg":"Reconciling opstree redis controller","Request.Namespace":"makro-acc","Request.Name":"redis-cache"}
{"level":"info","ts":1701246781.0988767,"logger":"controllers.Redis","msg":"Will reconcile redis operator in again 10 seconds","Request.Namespace":"makro-acc","Request.Name":"redis-cache"}
{"level":"info","ts":1701246781.0989869,"logger":"controllers.Redis","msg":"Reconciling opstree redis controller","Request.Namespace":"makro-acc","Request.Name":"redis-prometheus"}
{"level":"info","ts":1701246781.1679559,"logger":"controllers.Redis","msg":"Will reconcile redis operator in again 10 seconds","Request.Namespace":"makro-acc","Request.Name":"redis-prometheus"}

redis-operator version: image: 'quay.io/opstree/redis-operator:v0.15.1'

Does this issue reproduce with the latest release?

Yes

What did you do?

We upgraded to 0.15.9 from older version. We use beta1 CRDs. We replaced to the latest CRD definitions. We tried to experiment with switch beta1 to beta2 etc. On 'kubectl apply' CRD started to show:

Failed to load live state: conversion webhook for redis.redis.opstreelabs.in/v1beta2, Kind=Redis failed: Post "https://webhook-service.redis-operator.svc:443/convert?timeout=30s": dial tcp 10.108.151.1:443: connect: connection refused

We are not able to do anything with the CRDs as the conversion webhook fails and denies all operations.

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: v0.4.1
  name: redis.redis.redis.opstreelabs.in
  # omitted ...
spec:
  conversion:
    strategy: Webhook
    webhook: # THIS WEBHOOK is not enabled in the helm chart by default
      clientConfig:
        service:
          name: webhook-service
          namespace: redis-operator
          path: /convert
          port: 443
      conversionReviewVersions:
      - v1beta1
      - v1beta2
  group: redis.redis.opstreelabs.in

What did you expect to see?

I suggest 3 options:

  • either setup webhook and make it work out-of-the box for everyone
  • revise CRD spec.conversion and remove if not necessary
  • generate CRD dynamically based on the helm webhook configuration, so the .spec.conversion is left out in case of disabled webhook

What did you see instead?

Workaround for us - remove the CRDs from helm installation, patched them (removed converion part) and applied them separatedly.

Please note - as we use GitOps, the patching the CRD as suggested in the manual with TLS cert info is not viable, so without patching we get this error (of course):

  • lastTransitionTime: "2023-11-29T08:11:12Z" message: 'Failed to load live state: conversion webhook for redis.redis.opstreelabs.in/v1beta2, Kind=Redis failed: Post "https://webhook-service.redis-operator.svc:443/convert?timeout=30s": tls: failed to verify certificate: x509: certificate signed by unknown authority' type: ComparisonError

davidpechcz avatar Nov 29 '23 08:11 davidpechcz

I think we should remove the webhook conversion block from the helm CRD. would you like to open a PR ? We have move that to the docs here : https://github.com/OT-CONTAINER-KIT/helm-charts/blob/main/charts/redis-operator/readme.md

shubham-cmyk avatar Nov 30 '23 20:11 shubham-cmyk

I'd like to help, but can you explain please the reason for the conversion webhook? I mean it is safe to remove it without any other changes, or did you mean that is should be removed conditionally on some new helm chart variable?

davidpechcz avatar Dec 01 '23 05:12 davidpechcz

The reason we added the conversion webhook in the 1st place was to support the v1beta1 and v1beta2. If you use the v1beta2 version it's safe to remove that.

I found out that conversion part is still in the helm chart we should remove that.

shubham-cmyk avatar Dec 01 '23 05:12 shubham-cmyk

same questions, can you tell me how to remove it?

Praying avatar Dec 27 '23 07:12 Praying