diff --git a/pkg/controller/BUILD.bazel b/pkg/controller/BUILD.bazel index 187a98668c8d..26bdd46902d4 100644 --- a/pkg/controller/BUILD.bazel +++ b/pkg/controller/BUILD.bazel @@ -51,6 +51,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "conditions_test.go", "controller_ref_manager_test.go", "controller_suite_test.go", "expectations_test.go", diff --git a/pkg/controller/conditions.go b/pkg/controller/conditions.go index 59a02f5bb763..ea349a194b22 100644 --- a/pkg/controller/conditions.go +++ b/pkg/controller/conditions.go @@ -1,3 +1,21 @@ +/* + * This file is part of the KubeVirt project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright 2019 Red Hat, Inc. + * + */ package controller import ( diff --git a/pkg/controller/conditions_test.go b/pkg/controller/conditions_test.go new file mode 100644 index 000000000000..34fa8f3ccb27 --- /dev/null +++ b/pkg/controller/conditions_test.go @@ -0,0 +1,50 @@ +/* + * This file is part of the KubeVirt project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright 2019 Red Hat, Inc. + * + */ +package controller + +import ( + "testing" + + v12 "k8s.io/api/core/v1" + + v1 "kubevirt.io/client-go/api/v1" +) + +func TestAddPodCondition(t *testing.T) { + + vmi := v1.NewMinimalVMI("test") + + pc1 := &v12.PodCondition{ + Type: v12.PodScheduled, + Status: v12.ConditionFalse, + } + pc2 := &v12.PodCondition{ + Type: v12.PodScheduled, + Status: v12.ConditionTrue, + } + + cm := NewVirtualMachineInstanceConditionManager() + + cm.AddPodCondition(vmi, pc1) + cm.AddPodCondition(vmi, pc2) + + if len(vmi.Status.Conditions) != 1 { + t.Errorf("There should be exactly 1 condition when muliple conditions of the same type were added") + } +} diff --git a/pkg/virt-controller/watch/vmi.go b/pkg/virt-controller/watch/vmi.go index e0cf79b84c6f..597f1ac1704a 100644 --- a/pkg/virt-controller/watch/vmi.go +++ b/pkg/virt-controller/watch/vmi.go @@ -300,6 +300,10 @@ func (c *VMIController) updateStatus(vmi *virtv1.VirtualMachineInstance, pod *k8 Message: syncErr.Error(), Status: k8sv1.ConditionFalse, } + cm := controller.NewVirtualMachineInstanceConditionManager() + if cm.HasCondition(vmiCopy, condition.Type) { + cm.RemoveCondition(vmiCopy, condition.Type) + } vmiCopy.Status.Conditions = append(vmiCopy.Status.Conditions, condition) } } diff --git a/pkg/virt-controller/watch/vmi_test.go b/pkg/virt-controller/watch/vmi_test.go index c2e5a535d858..a347dc15baea 100644 --- a/pkg/virt-controller/watch/vmi_test.go +++ b/pkg/virt-controller/watch/vmi_test.go @@ -202,6 +202,7 @@ var _ = Describe("VirtualMachineInstance watcher", func() { vmiSource.Add(vmi) mockQueue.Wait() } + Context("On valid VirtualMachineInstance given with DataVolume source", func() { It("should create a corresponding Pod on VMI creation when DataVolume is ready", func() { @@ -457,6 +458,62 @@ var _ = Describe("VirtualMachineInstance watcher", func() { Expect(mockQueue.GetRateLimitedEnqueueCount()).To(Equal(0)) testutils.ExpectEvent(recorder, SuccessfulCreatePodReason) }) + It("should create PodScheduled and Synchronized conditions exactly once each for repeated FailedPvcNotFoundReason sync error", func() { + + expectConditions := func(vmi *v1.VirtualMachineInstance) { + // PodScheduled and Synchronized + Expect(len(vmi.Status.Conditions)).To(Equal(2), "there should be exactly 2 conditions") + + getType := func(c v1.VirtualMachineInstanceCondition) string { return string(c.Type) } + getReason := func(c v1.VirtualMachineInstanceCondition) string { return c.Reason } + Expect(vmi.Status.Conditions).To( + And( + ContainElement( + WithTransform(getType, Equal(string(v1.VirtualMachineInstanceSynchronized))), + ), + ContainElement( + And( + WithTransform(getType, Equal(string(k8sv1.PodScheduled))), + WithTransform(getReason, Equal(k8sv1.PodReasonUnschedulable)), + ), + ), + ), + ) + } + + vmi := NewPendingVirtualMachine("testvmi") + vmi.Spec.Volumes = []v1.Volume{ + { + Name: "test", + VolumeSource: v1.VolumeSource{ + PersistentVolumeClaim: &k8sv1.PersistentVolumeClaimVolumeSource{ + ClaimName: "something", + }, + }, + }, + } + addVirtualMachine(vmi) + update := vmiInterface.EXPECT().Update(gomock.Any()) + update.Do(func(vmi *v1.VirtualMachineInstance) { + expectConditions(vmi) + vmiInformer.GetStore().Update(vmi) + update.Return(vmi, nil) + }) + + controller.Execute() + Expect(controller.Queue.Len()).To(Equal(0)) + Expect(mockQueue.GetRateLimitedEnqueueCount()).To(Equal(1)) + + // make sure that during next iteration we do not add the same condition again + vmiInterface.EXPECT().Update(gomock.Any()).Do(func(vmi *v1.VirtualMachineInstance) { + expectConditions(vmi) + }).Return(vmi, nil) + + controller.Execute() + Expect(controller.Queue.Len()).To(Equal(0)) + Expect(mockQueue.GetRateLimitedEnqueueCount()).To(Equal(2)) + }) + table.DescribeTable("should move the vmi to scheduling state if a pod exists", func(phase k8sv1.PodPhase, isReady bool) { vmi := NewPendingVirtualMachine("testvmi") pod := NewPodForVirtualMachine(vmi, phase) diff --git a/tests/storage_test.go b/tests/storage_test.go index 136b0990d980..e7821940f63e 100644 --- a/tests/storage_test.go +++ b/tests/storage_test.go @@ -650,10 +650,24 @@ var _ = Describe("Storage", func() { if len(vmi.Status.Conditions) == 0 { return false } - Expect(vmi.Status.Conditions[0].Type).To(Equal(v1.VirtualMachineInstanceConditionType(k8sv1.PodScheduled))) - Expect(vmi.Status.Conditions[0].Reason).To(Equal(k8sv1.PodReasonUnschedulable)) - Expect(vmi.Status.Conditions[0].Status).To(Equal(k8sv1.ConditionFalse)) - Expect(vmi.Status.Conditions[0].Message).To(Equal(fmt.Sprintf("failed to render launch manifest: didn't find PVC %v", pvcName))) + + expectPodScheduledCondition := func(vmi *v1.VirtualMachineInstance) { + getType := func(c v1.VirtualMachineInstanceCondition) string { return string(c.Type) } + getReason := func(c v1.VirtualMachineInstanceCondition) string { return c.Reason } + getStatus := func(c v1.VirtualMachineInstanceCondition) k8sv1.ConditionStatus { return c.Status } + getMessage := func(c v1.VirtualMachineInstanceCondition) string { return c.Message } + Expect(vmi.Status.Conditions).To( + ContainElement( + And( + WithTransform(getType, Equal(string(k8sv1.PodScheduled))), + WithTransform(getReason, Equal(k8sv1.PodReasonUnschedulable)), + WithTransform(getStatus, Equal(k8sv1.ConditionFalse)), + WithTransform(getMessage, Equal(fmt.Sprintf("failed to render launch manifest: didn't find PVC %v", pvcName))), + ), + ), + ) + } + expectPodScheduledCondition(vmi) return true }, time.Duration(10)*time.Second).Should(BeTrue(), "Timed out waiting for VMI to get Unschedulable condition")