Skip to content

Commit

Permalink
UT Changes:
Browse files Browse the repository at this point in the history 
 - Refactoring:
  - Concentrating cleanup and teardown logic in the same place so maintenance would be easier
  - Removing redundant variables
  - Breaking Config and Controller into two independent suites removing redundant code from each suite initializer and sharing common code.
 - Adding test to cover new functionality

Signed-off-by: Michael Shitrit <[email protected]>
  • Loading branch information
@mshitrit
mshitrit committed Sep 13, 2023
1 parent 748e574 commit 6873211
Show file tree
Hide file tree
Showing 8 changed files with 555 additions and 213 deletions.
37 changes: 28 additions & 9 deletions ...fnoderemediationconfig_controller_test.go → ...fnoderemediationconfig_controller_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package controllers_test
package testconfig

import (
"context"
Expand All @@ -16,9 +16,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

selfnoderemediationv1alpha1 "github.com/medik8s/self-node-remediation/api/v1alpha1"
"github.com/medik8s/self-node-remediation/controllers/tests/shared"
)

var _ = Describe("snrc controller Test", func() {
var _ = Describe("SNR Config Test", func() {
dsName := "self-node-remediation-ds"
var config *selfnoderemediationv1alpha1.SelfNodeRemediationConfig
var ds *appsv1.DaemonSet
Expand All @@ -33,14 +34,14 @@ var _ = Describe("snrc controller Test", func() {
config.Spec.WatchdogFilePath = "/dev/foo"
config.Spec.SafeTimeToAssumeNodeRebootedSeconds = 123
config.Name = selfnoderemediationv1alpha1.ConfigCRName
config.Namespace = namespace
config.Namespace = shared.Namespace
config.Spec.HostPort = 30111

})

Context("DS installation", func() {
key := types.NamespacedName{
Namespace: namespace,
Namespace: shared.Namespace,
Name: dsName,
}

Expand Down Expand Up @@ -125,7 +126,25 @@ var _ = Describe("snrc controller Test", func() {

})
})
When("SafeTimeToAssumeNodeRebootedSeconds is modified in Configuration", func() {
BeforeEach(func() {
Expect(managerReconciler.SafeTimeCalculator.GetTimeToAssumeNodeRebooted()).ToNot(Equal(time.Minute))
config.Spec.SafeTimeToAssumeNodeRebootedSeconds = 60
})
It("The Manager Reconciler and the DS should be modified with the new value", func() {
Eventually(func() error {
return k8sClient.Get(context.Background(), key, ds)
}, 10*time.Second, 250*time.Millisecond).Should(BeNil())

dsContainers := ds.Spec.Template.Spec.Containers
Expect(len(dsContainers)).To(BeNumerically("==", 1))
container := dsContainers[0]
envVars := getEnvVarMap(container.Env)
Expect(envVars["TIME_TO_ASSUME_NODE_REBOOTED"].Value).To(Equal("60"))
Expect(managerReconciler.SafeTimeCalculator.GetTimeToAssumeNodeRebooted()).To(Equal(time.Minute))
})

})
Context("DS Recreation on Operator Update", func() {
var timeToWaitForDsUpdate = 6 * time.Second
var oldDsVersion, currentDsVersion = "0", "1"
Expand All @@ -139,7 +158,7 @@ var _ = Describe("snrc controller Test", func() {
})
When("ds version has not changed", func() {
BeforeEach(func() {
ds = generateDs(dsName, namespace, currentDsVersion)
ds = generateDs(dsName, shared.Namespace, currentDsVersion)
})
It("Daemonset should not recreated", func() {
//Wait to make sure DS isn't recreated
Expand All @@ -153,7 +172,7 @@ var _ = Describe("snrc controller Test", func() {
When("ds version has changed", func() {
BeforeEach(func() {
//creating an DS with old version
ds = generateDs(dsName, namespace, oldDsVersion)
ds = generateDs(dsName, shared.Namespace, oldDsVersion)
})
It("Daemonset should be recreated", func() {
//Wait until DS is recreated
Expand All @@ -175,7 +194,7 @@ var _ = Describe("snrc controller Test", func() {
config.Kind = "SelfNodeRemediationConfig"
config.APIVersion = "self-node-remediation.medik8s.io/v1alpha1"
config.Name = "config-defaults"
config.Namespace = namespace
config.Namespace = shared.Namespace

It("Config CR should be created with default values", func() {
Expect(k8sClient).To(Not(BeNil()))
Expand Down Expand Up @@ -211,7 +230,7 @@ var _ = Describe("snrc controller Test", func() {

ds = &appsv1.DaemonSet{}
key = types.NamespacedName{
Namespace: namespace,
Namespace: shared.Namespace,
Name: dsName,
}

Expand All @@ -226,7 +245,7 @@ var _ = Describe("snrc controller Test", func() {
config.Name = "not-the-expected-name"
config.Spec.WatchdogFilePath = "foo"
config.Spec.SafeTimeToAssumeNodeRebootedSeconds = 9999
config.Namespace = namespace
config.Namespace = shared.Namespace

Expect(k8sClient.Create(context.Background(), config)).To(Succeed())
})
Expand Down
154 changes: 60 additions & 94 deletions controllers/suite_test.go → controllers/tests/config/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers_test
package testconfig

import (
"context"
"errors"
"path/filepath"
"testing"
"time"
Expand All @@ -27,86 +26,43 @@ import (
. "github.com/onsi/gomega"

v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

selfnoderemediationv1alpha1 "github.com/medik8s/self-node-remediation/api/v1alpha1"
"github.com/medik8s/self-node-remediation/controllers"
"github.com/medik8s/self-node-remediation/controllers/tests/shared"
"github.com/medik8s/self-node-remediation/pkg/apicheck"
"github.com/medik8s/self-node-remediation/pkg/certificates"
"github.com/medik8s/self-node-remediation/pkg/peers"
"github.com/medik8s/self-node-remediation/pkg/reboot"
"github.com/medik8s/self-node-remediation/pkg/watchdog"
//+kubebuilder:scaffold:imports
)

// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var k8sClient *K8sClientWrapper
var testEnv *envtest.Environment
var dummyDog watchdog.Watchdog
var certReader certificates.CertStorageReader
var unhealthyNode = &v1.Node{}
var peerNode = &v1.Node{}
var unhealthyNode, peerNode = &v1.Node{}, &v1.Node{}
var cancelFunc context.CancelFunc

var unhealthyNodeNamespacedName = client.ObjectKey{
Name: unhealthyNodeName,
Namespace: "",
}
var peerNodeNamespacedName = client.ObjectKey{
Name: peerNodeName,
Namespace: "",
}

const (
peerUpdateInterval = 30 * time.Second
apiCheckInterval = 1 * time.Second
maxErrorThreshold = 1

namespace = "self-node-remediation"
unhealthyNodeName = "node1"
peerNodeName = "node2"
)

type K8sClientWrapper struct {
client.Client
Reader client.Reader
ShouldSimulateFailure bool
ShouldSimulateVaFailure bool
VaFailureMessage string
}

func (kcw *K8sClientWrapper) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
if kcw.ShouldSimulateFailure {
return errors.New("simulation of client error")
} else if kcw.ShouldSimulateVaFailure {
if _, ok := list.(*storagev1.VolumeAttachmentList); ok {
return errors.New(kcw.VaFailureMessage)
}
}
return kcw.Client.List(ctx, list, opts...)
}
var k8sClient *shared.K8sClientWrapper
var certReader certificates.CertStorageReader
var managerReconciler *controllers.SelfNodeRemediationReconciler

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecs(t, "Controller Suite")
RunSpecs(t, "SNR Config Test Suite")
}

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
CRDDirectoryPaths: []string{filepath.Join("../../..", "config", "crd", "bases")},
ErrorIfCRDPathMissing: true,
}

Expand All @@ -125,60 +81,50 @@ var _ = BeforeSuite(func() {
})
Expect(err).ToNot(HaveOccurred())

k8sClient = &K8sClientWrapper{
k8sManager.GetClient(),
k8sManager.GetAPIReader(),
false,
false,
"simulation of client error for VA",
k8sClient = &shared.K8sClientWrapper{
Client: k8sManager.GetClient(),
Reader: k8sManager.GetAPIReader(),
VaFailureMessage: "simulation of client error for VA",
}
Expect(k8sClient).ToNot(BeNil())

nsToCreate := &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Name: shared.Namespace,
},
}

Expect(k8sClient.Create(context.Background(), nsToCreate)).To(Succeed())

Expect(err).ToNot(HaveOccurred())
mockManagerCalculator := &mockCalculator{isAgent: false}
err = (&controllers.SelfNodeRemediationConfigReconciler{
Client: k8sManager.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-config-controller"),
InstallFileFolder: "../install/",
Scheme: scheme.Scheme,
Namespace: namespace,
Client: k8sManager.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-config-controller"),
InstallFileFolder: "../../../install/",
Scheme: scheme.Scheme,
Namespace: shared.Namespace,
ManagerSafeTimeCalculator: mockManagerCalculator,
}).SetupWithManager(k8sManager)

// peers need their own node on start
unhealthyNode = getNode(unhealthyNodeName)
unhealthyNode = getNode(shared.UnhealthyNodeName)
Expect(k8sClient.Create(context.Background(), unhealthyNode)).To(Succeed(), "failed to create unhealthy node")

peerNode = getNode(peerNodeName)
peerNode = getNode(shared.PeerNodeName)
Expect(k8sClient.Create(context.Background(), peerNode)).To(Succeed(), "failed to create peer node")

dummyDog, err = watchdog.NewFake(true)
Expect(err).ToNot(HaveOccurred())
err = k8sManager.Add(dummyDog)
Expect(err).ToNot(HaveOccurred())

peerApiServerTimeout := 5 * time.Second
peers := peers.New(unhealthyNodeName, peerUpdateInterval, k8sClient, ctrl.Log.WithName("peers"), peerApiServerTimeout)
peers := peers.New(shared.UnhealthyNodeName, shared.PeerUpdateInterval, k8sClient, ctrl.Log.WithName("peers"), peerApiServerTimeout)
err = k8sManager.Add(peers)
Expect(err).ToNot(HaveOccurred())

certReader = certificates.NewSecretCertStorage(k8sClient, ctrl.Log.WithName("SecretCertStorage"), namespace)
timeToAssumeNodeRebooted := time.Duration(maxErrorThreshold) * apiCheckInterval
timeToAssumeNodeRebooted += dummyDog.GetTimeout()
timeToAssumeNodeRebooted += 5 * time.Second
rebooter := reboot.NewWatchdogRebooter(dummyDog, ctrl.Log.WithName("rebooter"), &mockCalculator{mockTimeToAssumeNodeRebooted: timeToAssumeNodeRebooted})
certReader = certificates.NewSecretCertStorage(k8sClient, ctrl.Log.WithName("SecretCertStorage"), shared.Namespace)
apiConnectivityCheckConfig := &apicheck.ApiConnectivityCheckConfig{
Log: ctrl.Log.WithName("api-check"),
MyNodeName: unhealthyNodeName,
CheckInterval: apiCheckInterval,
MaxErrorsThreshold: maxErrorThreshold,
MyNodeName: shared.UnhealthyNodeName,
CheckInterval: shared.ApiCheckInterval,
MaxErrorsThreshold: shared.MaxErrorThreshold,
Peers: peers,
Rebooter: rebooter,
Cfg: cfg,
CertReader: certReader,
}
Expand All @@ -187,27 +133,38 @@ var _ = BeforeSuite(func() {
Expect(err).ToNot(HaveOccurred())

restoreNodeAfter := 5 * time.Second

mockAgentCalculator := &mockCalculator{isAgent: true}
// reconciler for unhealthy node
err = (&controllers.SelfNodeRemediationReconciler{
Client: k8sClient,
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-controller").WithName("unhealthy node"),
Rebooter: rebooter,
MyNodeName: unhealthyNodeName,
RestoreNodeAfter: restoreNodeAfter,
Client: k8sClient,
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-controller").WithName("unhealthy node"),
MyNodeName: shared.UnhealthyNodeName,
RestoreNodeAfter: restoreNodeAfter,
SafeTimeCalculator: mockAgentCalculator,
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

// reconciler for peer node
err = (&controllers.SelfNodeRemediationReconciler{
Client: k8sClient,
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-controller").WithName("peer node"),
MyNodeName: peerNodeName,
Rebooter: rebooter,
RestoreNodeAfter: restoreNodeAfter,
Client: k8sClient,
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-controller").WithName("peer node"),
MyNodeName: shared.PeerNodeName,
RestoreNodeAfter: restoreNodeAfter,
SafeTimeCalculator: mockAgentCalculator,
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

// reconciler for manager on peer node
managerReconciler = &controllers.SelfNodeRemediationReconciler{
Client: k8sClient,
Log: ctrl.Log.WithName("controllers").WithName("self-node-remediation-controller").WithName("manager node"),
MyNodeName: shared.PeerNodeName,
RestoreNodeAfter: restoreNodeAfter,
SafeTimeCalculator: mockManagerCalculator,
}
err = managerReconciler.SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

var ctx context.Context
ctx, cancelFunc = context.WithCancel(ctrl.SetupSignalHandler())

Expand All @@ -223,7 +180,7 @@ func getNode(name string) *v1.Node {
node := &v1.Node{}
node.Name = name
node.Labels = make(map[string]string)
node.Labels["kubernetes.io/hostname"] = unhealthyNodeName
node.Labels["kubernetes.io/hostname"] = name

return node
}
Expand All @@ -238,12 +195,21 @@ var _ = AfterSuite(func() {

type mockCalculator struct {
mockTimeToAssumeNodeRebooted time.Duration
isAgent bool
}

func (m *mockCalculator) GetTimeToAssumeNodeRebooted() time.Duration {
return m.mockTimeToAssumeNodeRebooted
}

func (m *mockCalculator) SetTimeToAssumeNodeRebooted(timeToAssumeNodeRebooted time.Duration) {
m.mockTimeToAssumeNodeRebooted = timeToAssumeNodeRebooted
}

func (m *mockCalculator) IsAgent() bool {
return m.isAgent
}

//goland:noinspection GoUnusedParameter
func (m *mockCalculator) Start(ctx context.Context) error {
return nil
Expand Down
Loading

0 comments on commit 6873211

Please sign in to comment.