Skip to content

Commit

Permalink
Validate updates to the whole .spec stanza (#913)
Browse files Browse the repository at this point in the history
HCO is supposed to validate updates to its CR
also delegating to components operators the validation
of the changes it has to propagate (trying to apply
them in dry run mode first and committing only if all the
involved components accepted it).

This was done only for the .spec.worloads stanza,
applying the same strategy to the whole .spec stanza.

Fixes: https://bugzilla.redhat.com/1893646

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

Co-authored-by: Simone Tiraboschi <stirabos@redhat.com>
  • Loading branch information
kubevirt-bot and tiraboschi authored Nov 16, 2020
1 parent 7aa0da0 commit 66b7b00
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 27 deletions.
5 changes: 3 additions & 2 deletions pkg/apis/hco/v1beta1/hyperconverged_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ func (r *HyperConverged) ValidateUpdate(old runtime.Object) error {
}

if !reflect.DeepEqual(
oldR.Spec.Workloads,
r.Spec.Workloads) {
oldR.Spec,
r.Spec) {

opts := &client.UpdateOptions{DryRun: []string{metav1.DryRunAll}}
for _, obj := range []runtime.Object{
r.NewKubeVirt(),
r.NewCDI(),
// TODO: try to validate with all the components
} {
if err := r.UpdateOperatorCr(ctx, obj, opts); err != nil {
return err
Expand Down
25 changes: 0 additions & 25 deletions pkg/apis/hco/v1beta1/hyperconverged_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,31 +155,6 @@ var _ = Describe("Hyperconverged Webhooks", func() {
Expect(err).Should(Equal(ErrFakeCdiError))
})

It("should not return error if no different in Spec.Workloads", func() {
hco := &HyperConverged{
Spec: HyperConvergedSpec{
Infra: HyperConvergedConfig{
NodePlacement: newHyperConvergedConfig(),
},
Workloads: HyperConvergedConfig{
NodePlacement: newHyperConvergedConfig(),
},
},
}

// replace the real client with a mock
cli = fake.NewFakeClientWithScheme(s, hco)

newHco := &HyperConverged{}
hco.DeepCopyInto(newHco)
// Change only infra, but leave workloads as is
newHco.Spec.Infra.NodePlacement.NodeSelector["a change"] = "Something else"

// should new return error, even when there are no CDI and KV
err := newHco.ValidateUpdate(hco)
Expect(err).To(BeNil())
})

It("should not return error if dry-run update of CDI CR passes", func() {
hco := &HyperConverged{
Spec: HyperConvergedSpec{
Expand Down

0 comments on commit 66b7b00

Please sign in to comment.