csi-addons / volume-replication-operator

Apache License 2.0
16 stars 23 forks source link

Create a webhook for volumereplication and volumereplicationclass #90

Closed iamniting closed 1 year ago

iamniting commented 3 years ago

Fixes #88, #89

Signed-off-by: Nitin Goyal nigoyal@redhat.com

iamniting commented 3 years ago

I have not tested this yet. Will give it a shot.

Madhu-1 commented 3 years ago

Please provide the logs and output once you finish with the testing.

iamniting commented 3 years ago

VRC webhook results when I am trying to change anything in the spec

$ oc edit vrc vrc 
error: volumereplicationclasses.replication.storage.openshift.io "vrc" could not be patched: admission webhook "vvolumereplicationclass.kb.io" denied the request: Spec.Provisioner is immutable
You can run `oc replace -f /tmp/oc-edit-cb67t.yaml` to try this update again.
$ oc edit vrc vrc 
error: volumereplicationclasses.replication.storage.openshift.io "vrc" could not be patched: admission webhook "vvolumereplicationclass.kb.io" denied the request: Spec.Parameters is immutable
You can run `oc replace -f /tmp/oc-edit-0brft.yaml` to try this update again.
iamniting commented 3 years ago

VR webhook is not working as expected as of now will debug and update

$ oc edit vr vr1
error: volumereplications.replication.storage.openshift.io "vr1" could not be patched: Internal error occurred: failed calling webhook "mvolumereplication.kb.io": webhook returned response.patchType but not response.patch
You can run `oc replace -f /tmp/oc-edit-3uojs.yaml` to try this update again.
iamniting commented 3 years ago

VR webhook is not working as expected as of now will debug and update

$ oc edit vr vr1
error: volumereplications.replication.storage.openshift.io "vr1" could not be patched: Internal error occurred: failed calling webhook "mvolumereplication.kb.io": webhook returned response.patchType but not response.patch
You can run `oc replace -f /tmp/oc-edit-3uojs.yaml` to try this update again.

Let me try this if it works https://github.com/operator-framework/operator-sdk/issues/4602

iamniting commented 3 years ago

Finally VR webhook is also working

$ oc edit vr vr1
error: volumereplications.replication.storage.openshift.io "vr1" could not be patched: admission webhook "vvolumereplication.kb.io" denied the request: Spec.DataSource is immutable
You can run `oc replace -f /tmp/oc-edit-3nzi3.yaml` to try this update again.
$ oc edit vr vr1
error: volumereplications.replication.storage.openshift.io "vr1" could not be patched: admission webhook "vvolumereplication.kb.io" denied the request: Spec.VolumeReplicationClass is immutable
You can run `oc replace -f /tmp/oc-edit-vuh8i.yaml` to try this update again.
iamniting commented 3 years ago

Update PROJECT config for 3alpha to 3 commit should be a separate PR as it is not specific to webhooks. This will help us track changes through PRs easily.

This can be done. I faced this issue while creating scafolding for webhook that's why updated in this one itself

VolumeReplication webhook scaffold and implementation should be a single commit. Same for VolumeReplicationClass webhook scaffold and implementation.

I would like to keep them as separate commits. It makes life easy to distinguish b/w generated code and implemented logic.

Controller runtime update should be a separate PR. Sometimes it has breaking changes which needs explicit communication.

I still think that we should send with this PR itself, because new controller runtime has a fix for webhook itself.

iamniting commented 3 years ago

How to test: once you have a working setup

change the image to CSI_VOLUME_REPLICATION_IMAGE: quay.io/nigoyal/volumereplication-operator:webhook kubectl edit configmaps rook-ceph-operator-config

create a cert manager kubectl apply -f https://github.com/jetstack/cert-manager/releases/download/v1.4.0/cert-manager.yaml

create certies, required service etc kubectl apply -f https://raw.githubusercontent.com/iamniting/conf/master/k8s/vr/webhook-conf.yaml --validate=false

now you can test the webhook functionality

mergify[bot] commented 3 years ago

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

iamniting commented 1 year ago

Closing this PR as it is not required anymore.