Skip to content

Commit

Permalink
don't allow creation of a VirtualMachineRestore if on is in progress
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Henriksen <[email protected]>
  • Loading branch information
mhenriks committed Sep 10, 2020
1 parent 1a9cb5f commit e4727f6
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 13 deletions.
8 changes: 6 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt
github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/RHsyseng/operator-utils v0.0.0-20190906175225-942a3f9c85a9/go.mod h1:E+hCtYz+9UsXfAGnRjX2LGuaa5gSGNKHCVTmGZR79vY=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/alexflint/go-filemutex v0.0.0-20171022225611-72bdc8eae2ae/go.mod h1:CgnQgUtFrFz9mxFNtED3jI5tLDjKlOM+oUF/sTk6ps0=
Expand Down Expand Up @@ -234,6 +235,8 @@ github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk=
github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.3.1 h1:qGJ6qTW+x6xX/my+8YUVl4WNpX9B7+/l2tRsHGZ7f2s=
github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y=
github.com/golang/protobuf v0.0.0-20161109072736-4bd1920723d7/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.0.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down Expand Up @@ -362,8 +365,6 @@ github.com/kubernetes-csi/csi-lib-utils v0.7.0/go.mod h1:bze+2G9+cmoHxN6+WyG1qT4
github.com/kubernetes-csi/csi-test v2.0.0+incompatible/go.mod h1:YxJ4UiuPWIhMBkxUKY5c267DyA0uDZ/MtAimhx/2TA0=
github.com/kubernetes-csi/external-snapshotter/v2 v2.1.1 h1:t5bmB3Y8nCaLA4aFrIpX0zjHEF/HUkJp6f5rm7BsVzM=
github.com/kubernetes-csi/external-snapshotter/v2 v2.1.1/go.mod h1:dV5oB3U62KBdlf9ADWkMmjGd3USauqQtwIm2OZb5mqI=
github.com/kubernetes-csi/external-snapshotter/v2 v2.0.1 h1:cRf1gQAzIXC6043qgLMfV3/LLddLmcqi5/c2bkuxaGI=
github.com/kubernetes-csi/external-snapshotter/v2 v2.0.1/go.mod h1:vUEcwbrEpsQ/rDgaO8WTe1gVIY/4CCj0S4Q+UuOq5wA=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
Expand Down Expand Up @@ -410,6 +411,8 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb
github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
github.com/onsi/ginkgo v0.0.0-20151202141238-7f8ab55aaf3b/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
Expand Down Expand Up @@ -588,6 +591,7 @@ golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTk
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE=
golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc=
golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down
1 change: 1 addition & 0 deletions manifests/generated/operator-csv.yaml.in
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ spec:
- snapshot.kubevirt.io
resources:
- virtualmachinesnapshots
- virtualmachinerestores
verbs:
- get
- list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ rules:
- snapshot.kubevirt.io
resources:
- virtualmachinesnapshots
- virtualmachinerestores
verbs:
- get
- list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,23 @@ func (admitter *VMRestoreAdmitter) Admit(ar *v1beta1.AdmissionReview) *v1beta1.A
return webhookutils.ToAdmissionResponseError(err)
}

restores, err := admitter.Client.VirtualMachineRestore(ar.Request.Namespace).List(metav1.ListOptions{})
if err != nil {
return webhookutils.ToAdmissionResponseError(err)
}

for _, r := range restores.Items {
if reflect.DeepEqual(r.Spec.Target, vmRestore.Spec.Target) &&
(r.Status == nil || r.Status.Complete == nil || *r.Status.Complete == false) {
cause := metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Message: fmt.Sprintf("VirtualMachineRestore %q in progress", r.Name),
Field: targetField.Child("name").String(),
}
causes = append(causes, cause)
}
}

causes = append(causes, snapshotCauses...)

case v1beta1.Update:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,15 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
Context("Without feature gate enabled", func() {
It("should reject anything", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{},
}

ar := createRestoreAdmissionReview(restore)
resp := createTestVMRestoreAdmitter(config, nil, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(resp.Result.Message).Should(Equal("Snapshot/Restore feature gate not enabled"))
})
Expand Down Expand Up @@ -108,13 +112,17 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
},
}

resp := createTestVMRestoreAdmitter(config, nil, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(resp.Result.Message).Should(ContainSubstring("Unexpected Resource"))
})

It("should reject missing apigroup", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
Kind: "VirtualMachine",
Expand All @@ -133,6 +141,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject when VM does not exist", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -152,6 +164,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject when VM and snapshot do not exist", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -163,7 +179,7 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
}

ar := createRestoreAdmissionReview(restore)
resp := createTestVMRestoreAdmitter(config, nil, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(len(resp.Result.Details.Causes)).To(Equal(2))
Expect(resp.Result.Details.Causes[0].Field).To(Equal("spec.target.name"))
Expand All @@ -172,6 +188,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject spec update", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -183,6 +203,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
}

oldRestore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -194,14 +218,18 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
}

ar := createRestoreUpdateAdmissionReview(oldRestore, restore)
resp := createTestVMRestoreAdmitter(config, nil, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(len(resp.Result.Details.Causes)).To(Equal(1))
Expect(resp.Result.Details.Causes[0].Field).To(Equal("spec"))
})

It("should allow metadata update", func() {
oldRestore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -213,6 +241,8 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
Finalizers: []string{"finalizer"},
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Expand All @@ -225,7 +255,7 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
}

ar := createRestoreUpdateAdmissionReview(oldRestore, restore)
resp := createTestVMRestoreAdmitter(config, nil, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
Expect(resp.Allowed).To(BeTrue())
})

Expand All @@ -243,6 +273,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject when VM is running", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -264,6 +298,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject when snapshot does not exist", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -277,14 +315,18 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
vm.Spec.Running = &f

ar := createRestoreAdmissionReview(restore)
resp := createTestVMRestoreAdmitter(config, vm, nil).Admit(ar)
resp := createTestVMRestoreAdmitter(config, vm).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(len(resp.Result.Details.Causes)).To(Equal(1))
Expect(resp.Result.Details.Causes[0].Field).To(Equal("spec.virtualMachineSnapshotName"))
})

It("should reject when snapshot not ready", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -308,6 +350,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject invalid kind", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -330,6 +376,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
It("should reject invalid apiGroup", func() {
g := "foo.bar"
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &g,
Expand All @@ -351,6 +401,10 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {

It("should reject invalid source ID", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Expand All @@ -370,6 +424,47 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
Expect(resp.Result.Details.Causes[0].Field).To(Equal("spec.virtualMachineSnapshotName"))
})

It("should reject if restore in progress", func() {
restore := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore-in-process",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Kind: "VirtualMachine",
Name: vmName,
},
VirtualMachineSnapshotName: vmSnapshotName,
},
}

f := false
vm.Spec.Running = &f

restoreInProcess := &snapshotv1.VirtualMachineRestore{
ObjectMeta: metav1.ObjectMeta{
Name: "restore-in-process",
Namespace: "default",
},
Spec: snapshotv1.VirtualMachineRestoreSpec{
Target: corev1.TypedLocalObjectReference{
APIGroup: &apiGroup,
Kind: "VirtualMachine",
Name: vmName,
},
VirtualMachineSnapshotName: vmSnapshotName,
},
}

ar := createRestoreAdmissionReview(restore)
resp := createTestVMRestoreAdmitter(config, vm, snapshot, restoreInProcess).Admit(ar)
Expect(resp.Allowed).To(BeFalse())
Expect(len(resp.Result.Details.Causes)).To(Equal(1))
Expect(resp.Result.Details.Causes[0].Field).To(Equal("spec.target.name"))
})

It("should accept when VM is not running", func() {
restore := &snapshotv1.VirtualMachineRestore{
Spec: snapshotv1.VirtualMachineRestoreSpec{
Expand Down Expand Up @@ -437,17 +532,19 @@ func createRestoreUpdateAdmissionReview(old, current *snapshotv1.VirtualMachineR
return ar
}

func createTestVMRestoreAdmitter(config *virtconfig.ClusterConfig, vm *v1.VirtualMachine, s *snapshotv1.VirtualMachineSnapshot) *VMRestoreAdmitter {
func createTestVMRestoreAdmitter(
config *virtconfig.ClusterConfig,
vm *v1.VirtualMachine,
objs ...runtime.Object,
) *VMRestoreAdmitter {
ctrl := gomock.NewController(GinkgoT())
virtClient := kubecli.NewMockKubevirtClient(ctrl)
vmInterface := kubecli.NewMockVirtualMachineInterface(ctrl)
var objs []runtime.Object
if s != nil {
objs = append(objs, s)
}
kubevirtClient := kubevirtfake.NewSimpleClientset(objs...)
virtClient.EXPECT().VirtualMachineSnapshot("default").
Return(kubevirtClient.SnapshotV1alpha1().VirtualMachineSnapshots("default"))
virtClient.EXPECT().VirtualMachineRestore("default").
Return(kubevirtClient.SnapshotV1alpha1().VirtualMachineRestores("default"))
virtClient.EXPECT().VirtualMachine(gomock.Any()).Return(vmInterface).AnyTimes()
if vm == nil {
err := errors.NewNotFound(schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachines"}, "foo")
Expand Down
1 change: 1 addition & 0 deletions pkg/virt-operator/creation/rbac/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func newApiServerClusterRole() *rbacv1.ClusterRole {
},
Resources: []string{
"virtualmachinesnapshots",
"virtualmachinerestores",
},
Verbs: []string{
"get", "list", "watch",
Expand Down
1 change: 1 addition & 0 deletions tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ go_test(
"//vendor/github.com/onsi/gomega/gstruct:go_default_library",
"//vendor/github.com/onsi/gomega/types:go_default_library",
"//vendor/github.com/pborman/uuid:go_default_library",
"//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library",
"//vendor/k8s.io/api/apps/v1:go_default_library",
"//vendor/k8s.io/api/authorization/v1:go_default_library",
"//vendor/k8s.io/api/autoscaling/v1:go_default_library",
Expand Down
Empty file added tests/k8s-reporter/1_events.log
Empty file.
Loading

0 comments on commit e4727f6

Please sign in to comment.