Skip to content

Commit

Permalink
Added owner reference for nns to node (#211)
Browse files Browse the repository at this point in the history
Thanks to that, NNS will be removed when respective Node is removed.
This patch also adds test coverage. Since we remove a Node as part of
the test, we run this test in a separate test lane.
  • Loading branch information
pitiK3U authored and phoracek committed Oct 11, 2019
1 parent d0c0659 commit ab0f4f1
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ E2E_TEST_ARGS ?= -test.v -test.timeout=40m -ginkgo.v -ginkgo.slowSpecThreshold=6
ifdef E2E_TEST_FOCUS
E2E_TEST_ARGS += -ginkgo.focus $(E2E_TEST_FOCUS)
endif
# Unless explicitly focused, always skip the cleanup test (it removes a node)
ifdef E2E_TEST_SKIP
E2E_TEST_ARGS += -ginkgo.skip $(E2E_TEST_SKIP)
E2E_TEST_ARGS += -ginkgo.skip .*NNS.*cleanup.*|$(E2E_TEST_SKIP)
else
E2E_TEST_ARGS += -ginkgo.skip .*NNS.*cleanup.*
endif
ifdef E2E_TEST_EXTRA_ARGS
E2E_TEST_ARGS += $(E2E_TEST_EXTRA_ARGS)
Expand Down
1 change: 1 addition & 0 deletions automation/check-patch.k8s-1.14.6.node-removal.mounts
1 change: 1 addition & 0 deletions automation/check-patch.k8s-1.14.6.node-removal.packages
1 change: 1 addition & 0 deletions automation/check-patch.k8s-1.14.6.node-removal.sh
1 change: 1 addition & 0 deletions automation/check-patch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ main() {
TARGET="${TARGET%.*}"
TARGET="${TARGET#*.}"
TARGET="${TARGET//[.]default-bridge/}"
TARGET="${TARGET//[.]node-removal/}"
echo "TARGET=$TARGET"
export TARGET
echo "Setup Go paths"
Expand Down
4 changes: 4 additions & 0 deletions automation/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ else
skip_tests=".*move.*default.*IP.*"
fi

if [[ $SCRIPT_NAME =~ node-removal ]]; then
focus_tests=".*NNS.*cleanup.*"
fi

make E2E_TEST_EXTRA_ARGS="$test_args" E2E_TEST_FOCUS="$focus_tests" E2E_TEST_SKIP="$skip_tests" test/e2e
6 changes: 2 additions & 4 deletions pkg/controller/node/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
return nmstate.EventIsForThisNode(createEvent.Meta)
},
DeleteFunc: func(event.DeleteEvent) bool {
return false // TODO: implement delete
return false
},
UpdateFunc: func(event.UpdateEvent) bool {
return false
Expand All @@ -61,7 +61,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
return false
},
}
//TODO: Watch deletes too handling it correctly at Reconciler
// Watch for changes to primary resource Node
err = c.Watch(&source.Kind{Type: &corev1.Node{}}, &handler.EnqueueRequestForObject{}, onCreationForThisNode)
if err != nil {
Expand Down Expand Up @@ -104,13 +103,12 @@ func (r *ReconcileNode) Reconcile(request reconcile.Request) (reconcile.Result,
return reconcile.Result{}, err
}

//TODO: Manage deletes
_, err = nmstate.GetNodeNetworkState(r.client, request.Name)
if err != nil {
if !errors.IsNotFound(err) {
return reconcile.Result{}, fmt.Errorf("error at node reconcile accessing NodeNetworkState: %v", err)
} else {
err = nmstate.InitializeNodeNeworkState(r.client, request.Name)
err = nmstate.InitializeNodeNeworkState(r.client, instance, r.scheme)
if err != nil {
return reconcile.Result{}, fmt.Errorf("error at node reconcile creating NodeNetworkState: %v", err)
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/helper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"strings"
"time"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -122,16 +124,20 @@ func GetNodeNetworkState(client client.Client, nodeName string) (nmstatev1alpha1
return nodeNetworkState, err
}

func InitializeNodeNeworkState(client client.Client, nodeName string) error {
func InitializeNodeNeworkState(client client.Client, node *corev1.Node, scheme *runtime.Scheme) error {
ownerRefList := []metav1.OwnerReference{{Name: node.ObjectMeta.Name, Kind: "Node", APIVersion: "v1", UID: node.UID}}

nodeNetworkState := nmstatev1alpha1.NodeNetworkState{
// Create NodeNetworkState for this node
ObjectMeta: metav1.ObjectMeta{
Name: nodeName,
Name: node.ObjectMeta.Name,
OwnerReferences: ownerRefList,
},
Spec: nmstatev1alpha1.NodeNetworkStateSpec{
NodeName: nodeName,
NodeName: node.ObjectMeta.Name,
},
}

err := client.Create(context.TODO(), &nodeNetworkState)
if err != nil {
return fmt.Errorf("error creating NodeNetworkState: %v, %+v", err, nodeNetworkState)
Expand Down
1 change: 1 addition & 0 deletions stdci.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
sub-stages:
- k8s-1.14.6.default-bridge
- k8s-1.14.6.node-removal
- k8s-1.14.6
- os-3.11.0

Expand Down
47 changes: 47 additions & 0 deletions test/e2e/nns_cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package e2e

import (
"context"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"

framework "github.com/operator-framework/operator-sdk/pkg/test"

nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
)

var _ = Describe("NNS cleanup", func() {
var nodeName types.NamespacedName

BeforeEach(func() {
nodeName = types.NamespacedName{Name: nodes[0]}

By("Checking that NNS exists")
Eventually(func() error {
return framework.Global.Client.Get(context.TODO(), nodeName, &nmstatev1alpha1.NodeNetworkState{})
}, ReadTimeout, ReadInterval).ShouldNot(HaveOccurred())
})

Context("after node removal", func() {
nodeToDelete := &corev1.Node{}

BeforeEach(func() {
By("Getting the node we want to delete")
err := framework.Global.Client.Get(context.TODO(), nodeName, nodeToDelete)
Expect(err).To(BeNil())

By("Deleting the node")
err = framework.Global.Client.Delete(context.TODO(), nodeToDelete)
Expect(err).To(BeNil())
})

It("should remove NNS of that node", func() {
Eventually(func() error {
return framework.Global.Client.Get(context.TODO(), nodeName, &nmstatev1alpha1.NodeNetworkState{})
}, ReadTimeout, ReadInterval).Should(HaveOccurred())
})
})
})

0 comments on commit ab0f4f1

Please sign in to comment.