Add webhook #151
Add webhook #151
Conversation
- Also, we make two jobs can run parallel Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Vicente-Cheng
force-pushed
the
add-webhook
branch
9 times, most recently
from
c969da7 to
f9b653d
Compare
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Vicente-Cheng
force-pushed
the
add-webhook
branch
from
f9b653d to
54c8c7c
Compare
|
Please review this PR. I use the new CI implementation (from PR: #150)
Please review the main logic about webhook and drop |
Vicente-Cheng
force-pushed
the
add-webhook
branch
from
54c8c7c to
ae631a4
Compare
Vicente-Cheng
force-pushed
the
add-webhook
branch
2 times, most recently
from
55b212e to
fa4393c
Compare
| Path: "/spec/provision", | ||
| Value: false, | ||
| } | ||
| patchOps = append(patchOps, patchProvision) |
There was a problem hiding this comment.
Should there also be a patch op here to explicitly remove Spec.FileSystem.Provisioned?
There was a problem hiding this comment.
No, it seems the admission mutate cannot update the deprecated part.
The controller now only checks the spec.provision, so we ignore this.
| // upgrade case, we need to update some fields | ||
| if device.Spec.Provisioner == nil && device.Status.ProvisionPhase == diskv1.ProvisionPhaseProvisioned { | ||
| device.Spec.Provision = true | ||
| device.Spec.FileSystem.Provisioned = false |
There was a problem hiding this comment.
ISTM that by setting Spec.Filesystem.Provisioned = false here, the mutator (which I assume runs later when the BD is subsequently updated?) will hit the prevProvision && (!newBd.Spec.Provision || !newBd.Spec.Filesystem.Provisioned) case, and will then go and patch in Spec.Provision = false, thus unprovisioning the device.
Or am I misunderstanding the order of events?
There was a problem hiding this comment.
Yeah, this might happen. So, the mutator will ignore the changes from the controller.
You can see we have the precheck as below:
if req.IsFromController() {
return nil, nil
}
After that, we could avoid the case you mentioned.
There was a problem hiding this comment.
Ah! Thank you, I had missed that detail.
Vicente-Cheng
force-pushed
the
add-webhook
branch
from
fa4393c to
d32fd09
Compare
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
- Now we only check the `spec.provision` Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Vicente-Cheng
force-pushed
the
add-webhook
branch
from
d32fd09 to
aefbba1
Compare
|
@Mergifyio backport v0.7.x |
✅ Backports have been created
|
Problem:
We need webhook to check and patch the blockdevice CR.
Solution:
Add webhook
Related Issue:
Test plan: