From 5dcdfa6bb442cd4abc5b2964c2154db806846fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20R=C3=B6hrich?= Date: Mon, 18 Dec 2023 14:04:20 +0100 Subject: [PATCH 1/2] ci: refactor node controller unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor node controller unit tests: - Break up single test function into multiple test functions - Create a clear separation between test case input data and expected result data - Implement existing tests with new structure as individual test functions - Avoid checking things that aren't subject matter of the test case at hand related-to: longhorn/longhorn#7332 Signed-off-by: Moritz Röhrich --- controller/controller_test.go | 17 + controller/node_controller_test.go | 1654 ++++++++++++++++++---------- 2 files changed, 1100 insertions(+), 571 deletions(-) diff --git a/controller/controller_test.go b/controller/controller_test.go index f0fabffa0c..7efdd82892 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -179,6 +179,23 @@ func newEngineImage(image string, state longhorn.EngineImageState) *longhorn.Eng } } +func newOrphan(spec longhorn.OrphanSpec, status longhorn.OrphanStatus) *longhorn.Orphan { + return &longhorn.Orphan{ + ObjectMeta: metav1.ObjectMeta{ + Name: types.GetOrphanChecksumNameForOrphanedDirectory( + spec.NodeID, + spec.Parameters[longhorn.OrphanDiskName], + spec.Parameters[longhorn.OrphanDiskPath], + spec.Parameters[longhorn.OrphanDiskUUID], + spec.Parameters[longhorn.OrphanDataName], + ), + Namespace: TestNamespace, + }, + Spec: spec, + Status: status, + } +} + func newEngineImageDaemonSet() *appsv1.DaemonSet { return &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controller/node_controller_test.go b/controller/node_controller_test.go index 450db8bd97..c82ea6d05c 100644 --- a/controller/node_controller_test.go +++ b/controller/node_controller_test.go @@ -2,132 +2,169 @@ package controller import ( "context" - "fmt" + monitor "github.com/longhorn/longhorn-manager/controller/monitor" + "github.com/longhorn/longhorn-manager/datastore" + longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" + lhfake "github.com/longhorn/longhorn-manager/k8s/pkg/client/clientset/versioned/fake" + "github.com/longhorn/longhorn-manager/types" + "github.com/longhorn/longhorn-manager/util" "github.com/sirupsen/logrus" - - "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" - "k8s.io/kubernetes/pkg/controller" - corev1 "k8s.io/api/core/v1" apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" - - "github.com/longhorn/longhorn-manager/datastore" - "github.com/longhorn/longhorn-manager/types" - "github.com/longhorn/longhorn-manager/util" - - monitor "github.com/longhorn/longhorn-manager/controller/monitor" - longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" - lhfake "github.com/longhorn/longhorn-manager/k8s/pkg/client/clientset/versioned/fake" + fake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + "k8s.io/kubernetes/pkg/controller" . "gopkg.in/check.v1" ) -const ( - ManagerPodUp = "managerPodUp" - ManagerPodDown = "managerPodDown" - KubeNodeDown = "kubeNodeDown" - KubeNodePressure = "kubeNodePressure" -) - var ( MountPropagationBidirectional = corev1.MountPropagationBidirectional -) -type NodeTestCase struct { - nodes map[string]*longhorn.Node - pods map[string]*corev1.Pod - replicas []*longhorn.Replica - kubeNodes map[string]*corev1.Node - instanceManagers map[string]*longhorn.InstanceManager + DefaultOrphanTestNode1 = newOrphan( + longhorn.OrphanSpec{ + NodeID: TestNode1, + Type: longhorn.OrphanTypeReplica, + Parameters: map[string]string{ + longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, + longhorn.OrphanDiskName: TestDiskID1, + longhorn.OrphanDiskUUID: TestDiskID1, + longhorn.OrphanDiskPath: TestDefaultDataPath, + }, + }, + longhorn.OrphanStatus{ + OwnerID: TestNode1, + }, + ) - expectNodeStatus map[string]longhorn.NodeStatus - expectInstanceManagers map[string]*longhorn.InstanceManager - expectOrphans []*longhorn.Orphan -} + DefaultInstanceManagerTestNode1 = newInstanceManager( + TestInstanceManagerName, + longhorn.InstanceManagerStateRunning, + TestOwnerID1, TestNode1, TestIP1, + map[string]longhorn.InstanceProcess{}, + map[string]longhorn.InstanceProcess{}, + false, + ) +) -func newTestNodeController(lhClient *lhfake.Clientset, kubeClient *fake.Clientset, extensionsClient *apiextensionsfake.Clientset, - informerFactories *util.InformerFactories, controllerID string) *NodeController { - ds := datastore.NewDataStore(TestNamespace, lhClient, kubeClient, extensionsClient, informerFactories) +// This data type contains all necessary fixtures for the test, like the mock +// API clients +type NodeControllerSuite struct { + kubeClient *fake.Clientset + lhClient *lhfake.Clientset + extensionsClient *apiextensionsfake.Clientset - logger := logrus.StandardLogger() - nc := NewNodeController(logger, ds, scheme.Scheme, kubeClient, TestNamespace, controllerID) - fakeRecorder := record.NewFakeRecorder(100) - nc.eventRecorder = fakeRecorder - nc.topologyLabelsChecker = fakeTopologyLabelsChecker + informerFactories *util.InformerFactories - enqueueNodeForMonitor := func(key string) { - nc.queue.Add(key) - } - mon, err := monitor.NewFakeNodeMonitor(nc.logger, nc.ds, controllerID, enqueueNodeForMonitor) - if err != nil { - return nil - } - nc.diskMonitor = mon + lhNodeIndexer cache.Indexer + lhReplicaIndexer cache.Indexer + lhSettingsIndexer cache.Indexer + lhInstanceManagerIndexer cache.Indexer + lhOrphanIndexer cache.Indexer - for index := range nc.cacheSyncs { - nc.cacheSyncs[index] = alwaysReady - } - return nc + podIndexer cache.Indexer + nodeIndexer cache.Indexer + + controller *NodeController } -func fakeTopologyLabelsChecker(kubeClient clientset.Interface, vers string) (bool, error) { - return false, nil +// This data type contains resource that exist in the cluster environment, like +// nodes and pods +type TestCase struct { + lhNodes map[string]*longhorn.Node + lhReplicas []*longhorn.Replica + lhSettings map[string]*longhorn.Setting + lhInstanceManagers map[string]*longhorn.InstanceManager + lhOrphans map[string]*longhorn.Orphan + pods map[string]*corev1.Pod + nodes map[string]*corev1.Node } -func generateKubeNodes(testType string) map[string]*corev1.Node { - var kubeNode1, kubeNode2 *corev1.Node - switch testType { - case KubeNodeDown: - kubeNode1 = newKubernetesNode(TestNode1, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - kubeNode2 = newKubernetesNode(TestNode2, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - case KubeNodePressure: - kubeNode1 = newKubernetesNode(TestNode1, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - kubeNode2 = newKubernetesNode(TestNode2, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - default: - kubeNode1 = newKubernetesNode(TestNode1, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - kubeNode2 = newKubernetesNode(TestNode2, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue) - } - return map[string]*corev1.Node{ - TestNode1: kubeNode1, - TestNode2: kubeNode2, - } +// This data type contains expected results in the form of resources. Each test +// will set up fixtures, input resources and expected results. Then it will +// initialize the mock API with initTest, execute a function of the controller +// and finally execute a set of assertions, which compare the actual contents of +// the mock API to the expected results +type Expectation struct { + nodeStatus map[string]*longhorn.NodeStatus + instanceManagers map[string]*longhorn.InstanceManager + orphans map[string]*longhorn.Orphan } -func generateManagerPod(testType string) map[string]*corev1.Pod { - var daemon1, daemon2 *corev1.Pod - switch testType { - case ManagerPodDown: - daemon1 = newDaemonPod(corev1.PodFailed, TestDaemon1, TestNamespace, TestNode1, TestIP1, nil) - daemon2 = newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, nil) - default: - daemon1 = newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional) - daemon2 = newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional) - } - return map[string]*corev1.Pod{ - TestDaemon1: daemon1, - TestDaemon2: daemon2, - } +var _ = Suite(&NodeControllerSuite{}) + +// This is setting up the NodeControllerSuite datastructure as a fixture. It is +// executed once before each test +func (s *NodeControllerSuite) SetUpTest(c *C) { + s.kubeClient = fake.NewSimpleClientset() + s.lhClient = lhfake.NewSimpleClientset() + s.extensionsClient = apiextensionsfake.NewSimpleClientset() + + s.informerFactories = util.NewInformerFactories(TestNamespace, s.kubeClient, s.lhClient, controller.NoResyncPeriodFunc()) + + s.lhNodeIndexer = s.informerFactories.LhInformerFactory.Longhorn().V1beta2().Nodes().Informer().GetIndexer() + s.lhReplicaIndexer = s.informerFactories.LhInformerFactory.Longhorn().V1beta2().Replicas().Informer().GetIndexer() + s.lhSettingsIndexer = s.informerFactories.LhInformerFactory.Longhorn().V1beta2().Settings().Informer().GetIndexer() + s.lhInstanceManagerIndexer = s.informerFactories.LhInformerFactory.Longhorn().V1beta2().InstanceManagers().Informer().GetIndexer() + s.lhOrphanIndexer = s.informerFactories.LhInformerFactory.Longhorn().V1beta2().Orphans().Informer().GetIndexer() + + s.podIndexer = s.informerFactories.KubeInformerFactory.Core().V1().Pods().Informer().GetIndexer() + s.nodeIndexer = s.informerFactories.KubeInformerFactory.Core().V1().Nodes().Informer().GetIndexer() + + s.controller = newTestNodeController(s.lhClient, s.kubeClient, s.extensionsClient, s.informerFactories, TestNode1) } -func kubeObjStatusSyncTest(testType string) *NodeTestCase { - tc := &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(testType) - node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") - node2 := newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, "") - nodes := map[string]*longhorn.Node{ - TestNode1: node1, - TestNode2: node2, - } - tc.nodes = nodes - nodeStatus := map[string]longhorn.NodeStatus{} - switch testType { - case ManagerPodUp: - nodeStatus = map[string]longhorn.NodeStatus{ +func (s *NodeControllerSuite) TestManagerPodUp(c *C) { + var err error + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, + } + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), @@ -141,9 +178,79 @@ func kubeObjStatusSyncTest(testType string) *NodeTestCase { newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), }, }, + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + } + + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) } - case ManagerPodDown: - nodeStatus = map[string]longhorn.NodeStatus{ + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) + } + + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestManagerPodDown(c *C) { + var err error + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodFailed, TestDaemon1, TestNamespace, TestNode1, TestIP1, nil), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, nil), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, + } + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), @@ -157,9 +264,79 @@ func kubeObjStatusSyncTest(testType string) *NodeTestCase { newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), }, }, + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + } + + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) } - case KubeNodeDown: - nodeStatus = map[string]longhorn.NodeStatus{ + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) + } + + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestKubeNodeDown(c *C) { + var err error + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, + } + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), @@ -173,9 +350,79 @@ func kubeObjStatusSyncTest(testType string) *NodeTestCase { newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), }, }, + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + } + + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) } - case KubeNodePressure: - nodeStatus = map[string]longhorn.NodeStatus{ + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) + } + + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestKubeNodePressure(c *C) { + var err error + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, + } + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), @@ -189,44 +436,36 @@ func kubeObjStatusSyncTest(testType string) *NodeTestCase { newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), }, }, - } + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, } - tc.pods = generateManagerPod(testType) - tc.expectNodeStatus = nodeStatus + s.initTest(c, tc) - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, - }, - }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, - }, + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - return tc + s.checkOrphans(c, tc, expectation) } -func (s *TestSuite) TestSyncNode(c *C) { - testCases := map[string]*NodeTestCase{} - testCases["manager pod up"] = kubeObjStatusSyncTest(ManagerPodUp) - testCases["manager pod down"] = kubeObjStatusSyncTest(ManagerPodDown) - testCases["kubernetes node down"] = kubeObjStatusSyncTest(KubeNodeDown) - testCases["kubernetes node pressure"] = kubeObjStatusSyncTest(KubeNodePressure) - - tc := &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusTrue, "") +func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -234,7 +473,7 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - node2 := newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusTrue, "") + node2 := newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node2.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -245,80 +484,125 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, - TestNode2: node2, + + vol := newVolume(TestVolumeName, 2) + eng := newEngineForVolume(vol) + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + TestNode2: node2, + }, + lhReplicas: []*longhorn.Replica{ + newReplicaForVolume(vol, eng, TestNode1, TestDiskID1), + newReplicaForVolume(vol, eng, TestNode2, TestDiskID2), + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, } - volume := newVolume(TestVolumeName, 2) - engine := newEngineForVolume(volume) - replica1 := newReplicaForVolume(volume, engine, TestNode1, TestDiskID1) - replica2 := newReplicaForVolume(volume, engine, TestNode2, TestDiskID2) - replicas := []*longhorn.Replica{replica1, replica2} - tc.replicas = replicas - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: TestVolumeSize, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), - }, - ScheduledReplica: map[string]int64{ - replica1.Name: replica1.Spec.VolumeSize, + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: TestVolumeSize, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), + }, + ScheduledReplica: map[string]int64{ + tc.lhReplicas[0].Name: tc.lhReplicas[0].Spec.VolumeSize, + }, + DiskUUID: TestDiskID1, + Type: longhorn.DiskTypeFilesystem, }, - DiskUUID: TestDiskID1, - Type: longhorn.DiskTypeFilesystem, }, }, - }, - TestNode2: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusUnknown, ""), + TestNode2: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusUnknown, ""), + }, + Type: longhorn.DiskTypeFilesystem, }, - Type: longhorn.DiskTypeFilesystem, }, }, }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, } - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, - }, - }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, - }, + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - testCases["only disk on node1 should be updated status"] = tc - tc = &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 = newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusTrue, "") + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -335,7 +619,7 @@ func (s *TestSuite) TestSyncNode(c *C) { }, }, } - node2 = newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusTrue, "") + node2 := newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node2.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -343,69 +627,114 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, - TestNode2: node2, + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + TestNode2: node2, + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, } - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), - newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), + newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + }, + ScheduledReplica: map[string]int64{}, + DiskUUID: TestDiskID1, + Type: longhorn.DiskTypeFilesystem, }, - ScheduledReplica: map[string]int64{}, - DiskUUID: TestDiskID1, - Type: longhorn.DiskTypeFilesystem, }, }, - }, - TestNode2: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Type: longhorn.DiskTypeFilesystem, + TestNode2: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Type: longhorn.DiskTypeFilesystem, + }, }, }, }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, } - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, - }, - }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, - }, + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - testCases["clean disk status when disk removed from the node spec"] = tc - tc = &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 = newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusTrue, "") + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Spec.Disks = map[string]longhorn.DiskSpec{ TestDiskID1: { Type: longhorn.DiskTypeFilesystem, @@ -427,7 +756,8 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - node2 = newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusTrue, "") + + node2 := newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node2.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -435,68 +765,114 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, - TestNode2: node2, + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + TestNode2: node2, + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, } - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskNotReady)), - newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskFilesystemChanged)), + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskNotReady)), + newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskFilesystemChanged)), + }, + ScheduledReplica: map[string]int64{}, + DiskUUID: "new-uuid", + Type: longhorn.DiskTypeFilesystem, }, - ScheduledReplica: map[string]int64{}, - DiskUUID: "new-uuid", - Type: longhorn.DiskTypeFilesystem, }, }, - }, - TestNode2: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Type: longhorn.DiskTypeFilesystem, + TestNode2: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), }, - }, - }, - } - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Type: longhorn.DiskTypeFilesystem, + }, }, }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + } + + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - testCases["test disable disk when file system changed"] = tc - tc = &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 = newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusTrue, "") + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -507,66 +883,110 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, } - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), - newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), + newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + }, + ScheduledReplica: map[string]int64{}, + DiskUUID: TestDiskID1, + Type: longhorn.DiskTypeFilesystem, }, - ScheduledReplica: map[string]int64{}, - DiskUUID: TestDiskID1, - Type: longhorn.DiskTypeFilesystem, }, }, }, + instanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: newInstanceManager( + TestInstanceManagerName, longhorn.InstanceManagerStateRunning, + TestOwnerID1, TestNode1, TestIP1, + map[string]longhorn.InstanceProcess{}, + map[string]longhorn.InstanceProcess{}, + false, + ), + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, } - tc.expectInstanceManagers = map[string]*longhorn.InstanceManager{ - TestInstanceManagerName: newInstanceManager( - TestInstanceManagerName, longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{}, - false, - ), - } + s.initTest(c, tc) - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, - }, - }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, - }, + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - testCases["create default instance managers after node up"] = tc - tc = &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 = newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusTrue, "") + s.checkInstanceManagers(c, tc, expectation) + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{ TestDiskID1: { StorageScheduled: 0, @@ -577,31 +997,7 @@ func (s *TestSuite) TestSyncNode(c *C) { Type: longhorn.DiskTypeFilesystem, }, } - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, - } - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{ - TestDiskID1: { - StorageScheduled: 0, - StorageAvailable: 0, - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), - newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), - }, - ScheduledReplica: map[string]int64{}, - DiskUUID: TestDiskID1, - Type: longhorn.DiskTypeFilesystem, - }, - }, - }, - } + extraInstanceManager := newInstanceManager( "extra-instance-manger-name", longhorn.InstanceManagerStateRunning, TestOwnerID1, TestNode1, TestIP1, @@ -621,233 +1017,349 @@ func (s *TestSuite) TestSyncNode(c *C) { ) extraInstanceManager.Spec.Image = TestExtraInstanceManagerImage - tc.instanceManagers = map[string]*longhorn.InstanceManager{ - TestInstanceManagerName: newInstanceManager( - TestInstanceManagerName, longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{}, - false, - ), - "extra-instance-manger-name": extraInstanceManager, - } - - tc.expectInstanceManagers = map[string]*longhorn.InstanceManager{ - TestInstanceManagerName: newInstanceManager( - TestInstanceManagerName, longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{}, - false, - ), - "extra-instance-manger-name": extraInstanceManager, - } - - tc.expectOrphans = []*longhorn.Orphan{ - { - Spec: longhorn.OrphanSpec{ - NodeID: TestNode1, - Type: longhorn.OrphanTypeReplica, - Parameters: map[string]string{ - longhorn.OrphanDataName: monitor.TestOrphanedReplicaDirectoryName, - longhorn.OrphanDiskName: TestDiskID1, - longhorn.OrphanDiskUUID: TestDiskID1, - longhorn.OrphanDiskPath: TestDefaultDataPath, + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + "extra-instance-manager-name": extraInstanceManager, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + }, + } + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), + }, + DiskStatus: map[string]*longhorn.DiskStatus{ + TestDiskID1: { + StorageScheduled: 0, + StorageAvailable: 0, + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), + newNodeCondition(longhorn.DiskConditionTypeReady, longhorn.ConditionStatusTrue, ""), + }, + ScheduledReplica: map[string]int64{}, + DiskUUID: TestDiskID1, + Type: longhorn.DiskTypeFilesystem, + }, }, }, - Status: longhorn.OrphanStatus{ - OwnerID: TestNode1, - }, + }, + instanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: newInstanceManager( + TestInstanceManagerName, longhorn.InstanceManagerStateRunning, + TestOwnerID1, TestNode1, TestIP1, + map[string]longhorn.InstanceProcess{}, + map[string]longhorn.InstanceProcess{}, + false, + ), + "extra-instance-manger-name": extraInstanceManager, + }, + orphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, }, } - testCases["clean up redundant instance managers only if there is no running instances"] = tc - tc = &NodeTestCase{} - tc.kubeNodes = generateKubeNodes(ManagerPodUp) - tc.pods = generateManagerPod(ManagerPodUp) - node1 = newNode(TestNode1, TestNamespace, false, longhorn.ConditionStatusTrue, "") + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) + } + + s.checkInstanceManagers(c, tc, expectation) + s.checkOrphans(c, tc, expectation) +} + +func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { + var err error + + node1 := newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, "") node1.Spec.Disks = map[string]longhorn.DiskSpec{} node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{} - tc.nodes = map[string]*longhorn.Node{ - TestNode1: node1, - } - tc.expectNodeStatus = map[string]longhorn.NodeStatus{ - TestNode1: { - Conditions: []longhorn.Condition{ - newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), - newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), - }, - DiskStatus: map[string]*longhorn.DiskStatus{}, + + tc := &TestCase{ + lhNodes: map[string]*longhorn.Node{ + TestNode1: node1, + }, + lhSettings: map[string]*longhorn.Setting{ + string(types.SettingNameDefaultInstanceManagerImage): newDefaultInstanceManagerImageSetting(), + }, + lhInstanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: DefaultInstanceManagerTestNode1, + }, + lhOrphans: map[string]*longhorn.Orphan{ + DefaultOrphanTestNode1.Name: DefaultOrphanTestNode1, + }, + pods: map[string]*corev1.Pod{ + TestDaemon1: newDaemonPod(corev1.PodRunning, TestDaemon1, TestNamespace, TestNode1, TestIP1, &MountPropagationBidirectional), + TestDaemon2: newDaemonPod(corev1.PodRunning, TestDaemon2, TestNamespace, TestNode2, TestIP2, &MountPropagationBidirectional), + }, + nodes: map[string]*corev1.Node{ + TestNode1: newKubernetesNode( + TestNode1, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), + TestNode2: newKubernetesNode( + TestNode2, + corev1.ConditionTrue, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionFalse, + corev1.ConditionTrue, + ), }, } - extraInstanceManager = newInstanceManager( - "extra-instance-manger-name", longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{ - ExistingInstance: { - Spec: longhorn.InstanceProcessSpec{ - Name: ExistingInstance, - }, - Status: longhorn.InstanceProcessStatus{ - State: longhorn.InstanceStateRunning, - PortStart: TestPort1, + + expectation := &Expectation{ + nodeStatus: map[string]*longhorn.NodeStatus{ + TestNode1: { + Conditions: []longhorn.Condition{ + newNodeCondition(longhorn.NodeConditionTypeSchedulable, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeReady, longhorn.ConditionStatusTrue, ""), + newNodeCondition(longhorn.NodeConditionTypeMountPropagation, longhorn.ConditionStatusTrue, ""), }, + DiskStatus: map[string]*longhorn.DiskStatus{}, }, - }, false) - extraInstanceManager.Spec.Image = TestExtraInstanceManagerImage - - tc.instanceManagers = map[string]*longhorn.InstanceManager{ - TestInstanceManagerName: newInstanceManager( - TestInstanceManagerName, longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{}, - false, - ), + }, + instanceManagers: map[string]*longhorn.InstanceManager{ + TestInstanceManagerName: newInstanceManager( + TestInstanceManagerName, longhorn.InstanceManagerStateRunning, + TestOwnerID1, TestNode1, TestIP1, + map[string]longhorn.InstanceProcess{}, + map[string]longhorn.InstanceProcess{}, + false, + ), + }, } - tc.expectInstanceManagers = map[string]*longhorn.InstanceManager{ - TestInstanceManagerName: newInstanceManager( - TestInstanceManagerName, longhorn.InstanceManagerStateRunning, - TestOwnerID1, TestNode1, TestIP1, - map[string]longhorn.InstanceProcess{}, - map[string]longhorn.InstanceProcess{}, - false, - ), + s.initTest(c, tc) + + for _, node := range tc.lhNodes { + if s.controller.controllerID == node.Name { + err = s.controller.diskMonitor.RunOnce() + c.Assert(err, IsNil) + } + + err = s.controller.syncNode(getKey(node, c)) + c.Assert(err, IsNil) + + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) + c.Assert(err, IsNil) + + s.checkNodeConditions(c, tc, expectation, n) } - tc.expectOrphans = []*longhorn.Orphan{} - testCases["clean up all instance managers if there is no disk on the node"] = tc - for name, tc := range testCases { - fmt.Printf("testing %v\n", name) + s.checkInstanceManagers(c, tc, expectation) +} - kubeClient := fake.NewSimpleClientset() - lhClient := lhfake.NewSimpleClientset() +// -- Helpers -- - informerFactories := util.NewInformerFactories(TestNamespace, kubeClient, lhClient, controller.NoResyncPeriodFunc()) +func (s *NodeControllerSuite) checkNodeConditions(c *C, tc *TestCase, exp *Expectation, node *longhorn.Node) { + // Check that all node status conditions match the expected node status + // conditions - save for the last transition timestamp and the actual + // message + for idx, condition := range node.Status.Conditions { + condition.LastTransitionTime = "" + condition.Message = "" + node.Status.Conditions[idx] = condition + } + c.Assert(node.Status.Conditions, DeepEquals, exp.nodeStatus[node.Name].Conditions) +} - nIndexer := informerFactories.LhInformerFactory.Longhorn().V1beta2().Nodes().Informer().GetIndexer() - pIndexer := informerFactories.KubeInformerFactory.Core().V1().Pods().Informer().GetIndexer() +func (s *NodeControllerSuite) checkDiskConditions(c *C, tc *TestCase, exp *Expectation, node *longhorn.Node) { + // Check that all disk status conditions match the expected disk status + // conditions - save for the last transition timestamp and the actual message + for fsid, diskStatus := range node.Status.DiskStatus { + for idx, condition := range diskStatus.Conditions { + if condition.Status != longhorn.ConditionStatusUnknown { + c.Assert(condition.LastTransitionTime, Not(Equals), "") + } + condition.LastTransitionTime = "" + condition.Message = "" + diskStatus.Conditions[idx] = condition + } + node.Status.DiskStatus[fsid] = diskStatus + } + c.Assert(node.Status.DiskStatus, DeepEquals, exp.nodeStatus[node.Name].DiskStatus) +} - rIndexer := informerFactories.LhInformerFactory.Longhorn().V1beta2().Replicas().Informer().GetIndexer() - knIndexer := informerFactories.KubeInformerFactory.Core().V1().Nodes().Informer().GetIndexer() +func (s *NodeControllerSuite) checkInstanceManagers(c *C, tc *TestCase, exp *Expectation) { + // Check that all existing instance managers are expected and all expected + // instance managers are existing + imList, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).List(context.TODO(), metav1.ListOptions{}) + c.Assert(err, IsNil) - sIndexer := informerFactories.LhInformerFactory.Longhorn().V1beta2().Settings().Informer().GetIndexer() - imIndexer := informerFactories.LhInformerFactory.Longhorn().V1beta2().InstanceManagers().Informer().GetIndexer() + for _, im := range imList.Items { + _, exists := exp.instanceManagers[im.Name] + c.Assert(exists, Equals, true) + } - imImageSetting := newDefaultInstanceManagerImageSetting() - imImageSetting, err := lhClient.LonghornV1beta2().Settings(TestNamespace).Create(context.TODO(), imImageSetting, metav1.CreateOptions{}) + for _, im := range exp.instanceManagers { + _, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Get(context.TODO(), im.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - err = sIndexer.Add(imImageSetting) + } +} + +func (s *NodeControllerSuite) checkOrphans(c *C, tc *TestCase, exp *Expectation) { + // Check that all existing orphans are expected and all expected orphans are + // existing + orphanList, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).List(context.TODO(), metav1.ListOptions{}) + c.Assert(err, IsNil) + + for _, orphan := range orphanList.Items { + _, exists := exp.orphans[orphan.Name] + c.Assert(exists, Equals, true) + } + + for _, expect := range exp.orphans { + _, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).Get(context.TODO(), expect.Name, metav1.GetOptions{}) c.Assert(err, IsNil) + } +} - // create kubernetes node - for _, kubeNode := range tc.kubeNodes { - n, err := kubeClient.CoreV1().Nodes().Create(context.TODO(), kubeNode, metav1.CreateOptions{}) - c.Assert(err, IsNil) - err = knIndexer.Add(n) - c.Assert(err, IsNil) +func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { + c.Assert(s.kubeClient, NotNil) + c.Assert(s.lhClient, NotNil) + c.Assert(s.extensionsClient, NotNil) - } + for _, node := range tc.lhNodes { + n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Create(context.TODO(), node, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(n, NotNil) + err = s.lhNodeIndexer.Add(n) + c.Assert(err, IsNil) + } - extensionsClient := apiextensionsfake.NewSimpleClientset() + for _, replica := range tc.lhReplicas { + r, err := s.lhClient.LonghornV1beta2().Replicas(TestNamespace).Create(context.TODO(), replica, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(r, NotNil) + err = s.lhReplicaIndexer.Add(r) + c.Assert(err, IsNil) + } - nc := newTestNodeController(lhClient, kubeClient, extensionsClient, informerFactories, TestNode1) + for _, setting := range tc.lhSettings { + set, err := s.lhClient.LonghornV1beta2().Settings(TestNamespace).Create(context.TODO(), setting, metav1.CreateOptions{}) c.Assert(err, IsNil) + c.Assert(set, NotNil) + err = s.lhSettingsIndexer.Add(set) + c.Assert(err, IsNil) + } - // create manager pod - for _, pod := range tc.pods { - p, err := kubeClient.CoreV1().Pods(TestNamespace).Create(context.TODO(), pod, metav1.CreateOptions{}) - c.Assert(err, IsNil) - err = pIndexer.Add(p) - c.Assert(err, IsNil) + for _, instanceManager := range tc.lhInstanceManagers { + im, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Create(context.TODO(), instanceManager, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(im, NotNil) + err = s.lhInstanceManagerIndexer.Add(im) + c.Assert(err, IsNil) + } - } - // create node - for _, node := range tc.nodes { - n, err := lhClient.LonghornV1beta2().Nodes(TestNamespace).Create(context.TODO(), node, metav1.CreateOptions{}) - c.Assert(err, IsNil) - c.Assert(n, NotNil) - err = nIndexer.Add(n) - c.Assert(err, IsNil) + for _, orphan := range tc.lhOrphans { + o, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).Create(context.TODO(), orphan, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(o, NotNil) + err = s.lhOrphanIndexer.Add(o) + c.Assert(err, IsNil) + } - } - // create replicas - for _, replica := range tc.replicas { - r, err := lhClient.LonghornV1beta2().Replicas(TestNamespace).Create(context.TODO(), replica, metav1.CreateOptions{}) - c.Assert(err, IsNil) - c.Assert(r, NotNil) - err = rIndexer.Add(r) - c.Assert(err, IsNil) - } - // create instance managers - for _, instanceManager := range tc.instanceManagers { - em, err := lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Create(context.TODO(), instanceManager, metav1.CreateOptions{}) - c.Assert(err, IsNil) - err = imIndexer.Add(em) - c.Assert(err, IsNil) - } - // sync node status - for nodeName, node := range tc.nodes { - if nc.controllerID == node.Name { - err = nc.diskMonitor.RunOnce() - c.Assert(err, IsNil) - } + for _, node := range tc.nodes { + n, err := s.kubeClient.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(n, NotNil) + err = s.nodeIndexer.Add(n) + c.Assert(err, IsNil) + } - err = nc.syncNode(getKey(node, c)) - c.Assert(err, IsNil) + for _, pod := range tc.pods { + p, err := s.kubeClient.CoreV1().Pods(TestNamespace).Create(context.TODO(), pod, metav1.CreateOptions{}) + c.Assert(err, IsNil) + c.Assert(p, NotNil) + err = s.podIndexer.Add(p) + c.Assert(err, IsNil) + } +} - n, err := lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) - c.Assert(err, IsNil) - for ctype, condition := range n.Status.Conditions { - condition.LastTransitionTime = "" - condition.Message = "" - n.Status.Conditions[ctype] = condition - } - c.Assert(n.Status.Conditions, DeepEquals, tc.expectNodeStatus[nodeName].Conditions) - if len(tc.expectNodeStatus[nodeName].DiskStatus) > 0 { - diskConditions := n.Status.DiskStatus - for fsid, diskStatus := range diskConditions { - for ctype, condition := range diskStatus.Conditions { - if condition.Status != longhorn.ConditionStatusUnknown { - c.Assert(condition.LastTransitionTime, Not(Equals), "") - } - condition.LastTransitionTime = "" - condition.Message = "" - diskStatus.Conditions[ctype] = condition - } - n.Status.DiskStatus[fsid] = diskStatus - } - c.Assert(n.Status.DiskStatus, DeepEquals, tc.expectNodeStatus[nodeName].DiskStatus) - } - } +func newTestNodeController(lhClient *lhfake.Clientset, kubeClient *fake.Clientset, extensionsClient *apiextensionsfake.Clientset, + informerFactories *util.InformerFactories, controllerID string) *NodeController { + ds := datastore.NewDataStore(TestNamespace, lhClient, kubeClient, extensionsClient, informerFactories) - for name := range tc.instanceManagers { - instanceManager, err := lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Get(context.TODO(), name, metav1.GetOptions{}) - c.Assert(err, IsNil) - if expectInstanceManager, exist := tc.expectInstanceManagers[name]; !exist { - c.Assert(datastore.ErrorIsNotFound(err), Equals, true) - } else { - c.Assert(instanceManager.Spec, DeepEquals, expectInstanceManager.Spec) - } - } + logger := logrus.StandardLogger() + nc := NewNodeController(logger, ds, scheme.Scheme, kubeClient, TestNamespace, controllerID) + fakeRecorder := record.NewFakeRecorder(100) + nc.eventRecorder = fakeRecorder + nc.topologyLabelsChecker = fakeTopologyLabelsChecker - if orphanList, err := lhClient.LonghornV1beta2().Orphans(TestNamespace).List(context.TODO(), metav1.ListOptions{}); err == nil { - c.Assert(len(orphanList.Items), Equals, len(tc.expectOrphans)) - } else { - c.Assert(len(tc.expectOrphans), Equals, 0) - } + enqueueNodeForMonitor := func(key string) { + nc.queue.Add(key) + } + mon, err := monitor.NewFakeNodeMonitor(nc.logger, nc.ds, controllerID, enqueueNodeForMonitor) + if err != nil { + return nil + } + nc.diskMonitor = mon - for _, orphan := range tc.expectOrphans { - orphanName := types.GetOrphanChecksumNameForOrphanedDirectory(orphan.Spec.NodeID, - orphan.Spec.Parameters[longhorn.OrphanDiskName], - orphan.Spec.Parameters[longhorn.OrphanDiskPath], - orphan.Spec.Parameters[longhorn.OrphanDiskUUID], - orphan.Spec.Parameters[longhorn.OrphanDataName]) - _, err := lhClient.LonghornV1beta2().Orphans(TestNamespace).Get(context.TODO(), orphanName, metav1.GetOptions{}) - c.Assert(err, IsNil) - } + for index := range nc.cacheSyncs { + nc.cacheSyncs[index] = alwaysReady } + return nc +} + +func fakeTopologyLabelsChecker(kubeClient clientset.Interface, vers string) (bool, error) { + return false, nil } From f9344c71ac25ea190fea4b9bdff375bff9bf2f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20R=C3=B6hrich?= Date: Tue, 19 Dec 2023 14:14:34 +0100 Subject: [PATCH 2/2] ci: improve node controller tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename "TestCase" to "NodeControllerFixture" - Rename "Expectation" to "NodeControllerExpectation" - Avoid passing unnecessary parameters to test helper functions - Use "checkDiskConditions" test helper function in all places where disk conditions need to be tested Signed-off-by: Moritz Röhrich --- controller/node_controller_test.go | 171 +++++++++++++++-------------- 1 file changed, 88 insertions(+), 83 deletions(-) diff --git a/controller/node_controller_test.go b/controller/node_controller_test.go index c82ea6d05c..8d5e3e06fb 100644 --- a/controller/node_controller_test.go +++ b/controller/node_controller_test.go @@ -75,7 +75,7 @@ type NodeControllerSuite struct { // This data type contains resource that exist in the cluster environment, like // nodes and pods -type TestCase struct { +type NodeControllerFixture struct { lhNodes map[string]*longhorn.Node lhReplicas []*longhorn.Replica lhSettings map[string]*longhorn.Setting @@ -90,7 +90,7 @@ type TestCase struct { // initialize the mock API with initTest, execute a function of the controller // and finally execute a set of assertions, which compare the actual contents of // the mock API to the expected results -type Expectation struct { +type NodeControllerExpectation struct { nodeStatus map[string]*longhorn.NodeStatus instanceManagers map[string]*longhorn.InstanceManager orphans map[string]*longhorn.Orphan @@ -122,7 +122,7 @@ func (s *NodeControllerSuite) SetUpTest(c *C) { func (s *NodeControllerSuite) TestManagerPodUp(c *C) { var err error - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), @@ -163,7 +163,7 @@ func (s *NodeControllerSuite) TestManagerPodUp(c *C) { ), }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -184,9 +184,9 @@ func (s *NodeControllerSuite) TestManagerPodUp(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -198,16 +198,16 @@ func (s *NodeControllerSuite) TestManagerPodUp(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestManagerPodDown(c *C) { var err error - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), @@ -249,7 +249,7 @@ func (s *NodeControllerSuite) TestManagerPodDown(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -270,9 +270,9 @@ func (s *NodeControllerSuite) TestManagerPodDown(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -284,16 +284,16 @@ func (s *NodeControllerSuite) TestManagerPodDown(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestKubeNodeDown(c *C) { var err error - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), @@ -335,7 +335,7 @@ func (s *NodeControllerSuite) TestKubeNodeDown(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -356,9 +356,9 @@ func (s *NodeControllerSuite) TestKubeNodeDown(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -370,16 +370,16 @@ func (s *NodeControllerSuite) TestKubeNodeDown(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestKubeNodePressure(c *C) { var err error - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: newNode(TestNode1, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), TestNode2: newNode(TestNode2, TestNamespace, true, longhorn.ConditionStatusUnknown, ""), @@ -421,7 +421,7 @@ func (s *NodeControllerSuite) TestKubeNodePressure(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -442,9 +442,9 @@ func (s *NodeControllerSuite) TestKubeNodePressure(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -456,10 +456,10 @@ func (s *NodeControllerSuite) TestKubeNodePressure(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { @@ -488,7 +488,7 @@ func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { vol := newVolume(TestVolumeName, 2) eng := newEngineForVolume(vol) - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, TestNode2: node2, @@ -534,7 +534,7 @@ func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -550,7 +550,7 @@ func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { newNodeCondition(longhorn.DiskConditionTypeSchedulable, longhorn.ConditionStatusFalse, string(longhorn.DiskConditionReasonDiskPressure)), }, ScheduledReplica: map[string]int64{ - tc.lhReplicas[0].Name: tc.lhReplicas[0].Spec.VolumeSize, + fixture.lhReplicas[0].Name: fixture.lhReplicas[0].Spec.VolumeSize, }, DiskUUID: TestDiskID1, Type: longhorn.DiskTypeFilesystem, @@ -579,9 +579,9 @@ func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -593,10 +593,11 @@ func (s *NodeControllerSuite) TestUpdateDiskStatus(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) + s.checkDiskConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { @@ -628,7 +629,7 @@ func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { }, } - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, TestNode2: node2, @@ -670,7 +671,7 @@ func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -711,9 +712,9 @@ func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -725,10 +726,11 @@ func (s *NodeControllerSuite) TestCleanDiskStatus(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) + s.checkDiskConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { @@ -766,7 +768,7 @@ func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { }, } - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, TestNode2: node2, @@ -808,7 +810,7 @@ func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -849,9 +851,9 @@ func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -863,10 +865,11 @@ func (s *NodeControllerSuite) TestDisableDiskOnFilesystemChange(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) + s.checkDiskConditions(c, expectation, n) } - s.checkOrphans(c, tc, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { @@ -884,7 +887,7 @@ func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { }, } - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, }, @@ -925,7 +928,7 @@ func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -962,9 +965,9 @@ func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -976,11 +979,12 @@ func (s *NodeControllerSuite) TestCreateDefaultInstanceManager(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) + s.checkDiskConditions(c, expectation, n) } - s.checkInstanceManagers(c, tc, expectation) - s.checkOrphans(c, tc, expectation) + s.checkInstanceManagers(c, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { @@ -1017,7 +1021,7 @@ func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { ) extraInstanceManager.Spec.Image = TestExtraInstanceManagerImage - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, }, @@ -1059,7 +1063,7 @@ func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -1097,9 +1101,9 @@ func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -1111,11 +1115,12 @@ func (s *NodeControllerSuite) TestCleanupRedundantInstanceManagers(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) + s.checkDiskConditions(c, expectation, n) } - s.checkInstanceManagers(c, tc, expectation) - s.checkOrphans(c, tc, expectation) + s.checkInstanceManagers(c, expectation) + s.checkOrphans(c, expectation) } func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { @@ -1125,7 +1130,7 @@ func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { node1.Spec.Disks = map[string]longhorn.DiskSpec{} node1.Status.DiskStatus = map[string]*longhorn.DiskStatus{} - tc := &TestCase{ + fixture := &NodeControllerFixture{ lhNodes: map[string]*longhorn.Node{ TestNode1: node1, }, @@ -1166,7 +1171,7 @@ func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { }, } - expectation := &Expectation{ + expectation := &NodeControllerExpectation{ nodeStatus: map[string]*longhorn.NodeStatus{ TestNode1: { Conditions: []longhorn.Condition{ @@ -1188,9 +1193,9 @@ func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { }, } - s.initTest(c, tc) + s.initTest(c, fixture) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { if s.controller.controllerID == node.Name { err = s.controller.diskMonitor.RunOnce() c.Assert(err, IsNil) @@ -1202,15 +1207,15 @@ func (s *NodeControllerSuite) TestCleanupAllInstanceManagers(c *C) { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Get(context.TODO(), node.Name, metav1.GetOptions{}) c.Assert(err, IsNil) - s.checkNodeConditions(c, tc, expectation, n) + s.checkNodeConditions(c, expectation, n) } - s.checkInstanceManagers(c, tc, expectation) + s.checkInstanceManagers(c, expectation) } // -- Helpers -- -func (s *NodeControllerSuite) checkNodeConditions(c *C, tc *TestCase, exp *Expectation, node *longhorn.Node) { +func (s *NodeControllerSuite) checkNodeConditions(c *C, expectation *NodeControllerExpectation, node *longhorn.Node) { // Check that all node status conditions match the expected node status // conditions - save for the last transition timestamp and the actual // message @@ -1219,10 +1224,10 @@ func (s *NodeControllerSuite) checkNodeConditions(c *C, tc *TestCase, exp *Expec condition.Message = "" node.Status.Conditions[idx] = condition } - c.Assert(node.Status.Conditions, DeepEquals, exp.nodeStatus[node.Name].Conditions) + c.Assert(node.Status.Conditions, DeepEquals, expectation.nodeStatus[node.Name].Conditions) } -func (s *NodeControllerSuite) checkDiskConditions(c *C, tc *TestCase, exp *Expectation, node *longhorn.Node) { +func (s *NodeControllerSuite) checkDiskConditions(c *C, expectation *NodeControllerExpectation, node *longhorn.Node) { // Check that all disk status conditions match the expected disk status // conditions - save for the last transition timestamp and the actual message for fsid, diskStatus := range node.Status.DiskStatus { @@ -1236,49 +1241,49 @@ func (s *NodeControllerSuite) checkDiskConditions(c *C, tc *TestCase, exp *Expec } node.Status.DiskStatus[fsid] = diskStatus } - c.Assert(node.Status.DiskStatus, DeepEquals, exp.nodeStatus[node.Name].DiskStatus) + c.Assert(node.Status.DiskStatus, DeepEquals, expectation.nodeStatus[node.Name].DiskStatus) } -func (s *NodeControllerSuite) checkInstanceManagers(c *C, tc *TestCase, exp *Expectation) { +func (s *NodeControllerSuite) checkInstanceManagers(c *C, expectation *NodeControllerExpectation) { // Check that all existing instance managers are expected and all expected // instance managers are existing imList, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).List(context.TODO(), metav1.ListOptions{}) c.Assert(err, IsNil) for _, im := range imList.Items { - _, exists := exp.instanceManagers[im.Name] + _, exists := expectation.instanceManagers[im.Name] c.Assert(exists, Equals, true) } - for _, im := range exp.instanceManagers { + for _, im := range expectation.instanceManagers { _, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Get(context.TODO(), im.Name, metav1.GetOptions{}) c.Assert(err, IsNil) } } -func (s *NodeControllerSuite) checkOrphans(c *C, tc *TestCase, exp *Expectation) { +func (s *NodeControllerSuite) checkOrphans(c *C, expectation *NodeControllerExpectation) { // Check that all existing orphans are expected and all expected orphans are // existing orphanList, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).List(context.TODO(), metav1.ListOptions{}) c.Assert(err, IsNil) for _, orphan := range orphanList.Items { - _, exists := exp.orphans[orphan.Name] + _, exists := expectation.orphans[orphan.Name] c.Assert(exists, Equals, true) } - for _, expect := range exp.orphans { + for _, expect := range expectation.orphans { _, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).Get(context.TODO(), expect.Name, metav1.GetOptions{}) c.Assert(err, IsNil) } } -func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { +func (s *NodeControllerSuite) initTest(c *C, fixture *NodeControllerFixture) { c.Assert(s.kubeClient, NotNil) c.Assert(s.lhClient, NotNil) c.Assert(s.extensionsClient, NotNil) - for _, node := range tc.lhNodes { + for _, node := range fixture.lhNodes { n, err := s.lhClient.LonghornV1beta2().Nodes(TestNamespace).Create(context.TODO(), node, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(n, NotNil) @@ -1286,7 +1291,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, replica := range tc.lhReplicas { + for _, replica := range fixture.lhReplicas { r, err := s.lhClient.LonghornV1beta2().Replicas(TestNamespace).Create(context.TODO(), replica, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(r, NotNil) @@ -1294,7 +1299,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, setting := range tc.lhSettings { + for _, setting := range fixture.lhSettings { set, err := s.lhClient.LonghornV1beta2().Settings(TestNamespace).Create(context.TODO(), setting, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(set, NotNil) @@ -1302,7 +1307,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, instanceManager := range tc.lhInstanceManagers { + for _, instanceManager := range fixture.lhInstanceManagers { im, err := s.lhClient.LonghornV1beta2().InstanceManagers(TestNamespace).Create(context.TODO(), instanceManager, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(im, NotNil) @@ -1310,7 +1315,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, orphan := range tc.lhOrphans { + for _, orphan := range fixture.lhOrphans { o, err := s.lhClient.LonghornV1beta2().Orphans(TestNamespace).Create(context.TODO(), orphan, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(o, NotNil) @@ -1318,7 +1323,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, node := range tc.nodes { + for _, node := range fixture.nodes { n, err := s.kubeClient.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(n, NotNil) @@ -1326,7 +1331,7 @@ func (s *NodeControllerSuite) initTest(c *C, tc *TestCase) { c.Assert(err, IsNil) } - for _, pod := range tc.pods { + for _, pod := range fixture.pods { p, err := s.kubeClient.CoreV1().Pods(TestNamespace).Create(context.TODO(), pod, metav1.CreateOptions{}) c.Assert(err, IsNil) c.Assert(p, NotNil)