Ping default gw from withing the cluster to verify connectivity (#202)

In case it fails it will rollback nmstate changes.

Signed-off-by: Quique Llorente <[email protected]>

build/Dockerfile

@@ -1,6 +1,6 @@
 FROM fedora:30
 
-RUN sudo dnf install -y nmstate iproute && \
+RUN sudo dnf install -y nmstate iproute iputils && \
     sudo dnf clean all
 
 # TODO: Delete this line after we update nmstate to include the change

pkg/helper/client.go

@@ -14,6 +14,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	yaml "sigs.k8s.io/yaml"
 
+	"github.com/tidwall/gjson"
+
 	"github.com/gobwas/glob"
 	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
 )
@@ -159,6 +161,34 @@ func UpdateCurrentState(client client.Client, nodeNetworkState *nmstatev1alpha1.
 	return nil
 }
 
+func ping(target string) (string, error) {
+	cmd := exec.Command("ping", "-c", "3", target)
+	var outputBuffer bytes.Buffer
+	cmd.Stdout = &outputBuffer
+	cmd.Stderr = &outputBuffer
+	return outputBuffer.String(), cmd.Run()
+}
+
+func defaultGw() (string, error) {
+	observedStateRaw, err := show()
+	if err != nil {
+		return "", fmt.Errorf("error running nmstatectl show: %v", err)
+	}
+
+	currentState, err := yaml.YAMLToJSON([]byte(observedStateRaw))
+	if err != nil {
+		return "", fmt.Errorf("Impossible to convert current state to JSON")
+	}
+
+	defaultGw := gjson.ParseBytes([]byte(currentState)).
+		Get("routes.running.#(destination==\"0.0.0.0/0\").next-hop-address").String()
+	if defaultGw == "" {
+		return "", fmt.Errorf("Impossible to retrieve default gw")
+	}
+
+	return defaultGw, nil
+}
+
 func ApplyDesiredState(nodeNetworkState *nmstatev1alpha1.NodeNetworkState) (string, error) {
 	desiredState := string(nodeNetworkState.Spec.DesiredState)
 	if len(desiredState) == 0 {
@@ -188,10 +218,21 @@ func ApplyDesiredState(nodeNetworkState *nmstatev1alpha1.NodeNetworkState) (stri
 		}
 	}
 
+	defaultGw, err := defaultGw()
+	if err != nil {
+		return commandOutput, rollback(err)
+	}
+
+	pingOutput, err := ping(defaultGw)
+	if err != nil {
+		return pingOutput, rollback(fmt.Errorf("error pinging external address after network reconfiguration: %v", err))
+	}
+
 	_, err = commit()
 	if err != nil {
 		return commandOutput, rollback(err)
 	}
+
 	commandOutput += fmt.Sprintf("setOutput: %s \n", setOutput)
 	return commandOutput, nil
 }

test/e2e/default_bridged_network_test.go

@@ -2,16 +2,13 @@ package e2e
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	"github.com/tidwall/gjson"
 
-	yaml "sigs.k8s.io/yaml"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 
@@ -118,31 +115,13 @@ var _ = Describe("NodeNetworkConfigurationPolicy default bridged network", func(
 	})
 })
 
-func currentStateJSON(node string) []byte {
-	key := types.NamespacedName{Name: node}
-	currentState := nodeNetworkState(key).Status.CurrentState
-	currentStateJson, err := yaml.YAMLToJSON([]byte(currentState))
-	ExpectWithOffset(1, err).ToNot(HaveOccurred())
-	return currentStateJson
-}
-
-func ipv4Address(node string, name string) string {
-	path := fmt.Sprintf("interfaces.#(name==\"%s\").ipv4.address.0.ip", name)
-	return gjson.ParseBytes(currentStateJSON(node)).Get(path).String()
-}
-
 func defaultRouteNextHopInterface(node string) AsyncAssertion {
 	return Eventually(func() string {
 		path := "routes.running.#(destination==\"0.0.0.0/0\").next-hop-interface"
 		return gjson.ParseBytes(currentStateJSON(node)).Get(path).String()
 	}, 15*time.Second, 1*time.Second)
 }
 
-func dhcpFlag(node string, name string) bool {
-	path := fmt.Sprintf("interfaces.#(name==\"%s\").ipv4.dhcp", name)
-	return gjson.ParseBytes(currentStateJSON(node)).Get(path).Bool()
-}
-
 func nodeReadyConditionStatus(nodeName string) (corev1.ConditionStatus, error) {
 	key := types.NamespacedName{Name: nodeName}
 	node := corev1.Node{}

test/e2e/rollback_test.go

@@ -5,8 +5,30 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+
+	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
 )
 
+func badDefaultGw(address string, nic string) nmstatev1alpha1.State {
+	return nmstatev1alpha1.State(fmt.Sprintf(`interfaces:
+  - name: %s
+    type: ethernet
+    state: up
+    ipv4:
+      dhcp: false
+      enabled: true
+      address:
+        - ip: %s
+          prefix-length: 24
+routes:
+  config:
+    - destination: 0.0.0.0/0
+      metric: 150
+      next-hop-address: 192.0.2.1
+      next-hop-interface: %s
+`, nic, address, nic))
+}
+
 var _ = Describe("rollback", func() {
 	Context("when an error happens during state configuration", func() {
 		BeforeEach(func() {
@@ -26,11 +48,41 @@ var _ = Describe("rollback", func() {
 			for _, node := range nodes {
 				By(fmt.Sprintf("Check that %s has being rolled back", bridge1))
 				interfacesNameForNodeEventually(node).ShouldNot(ContainElement(bridge1))
-				By("Check reconcile re-apply desiredState")
+				By("Check that desiredState is applied")
 				interfacesNameForNodeEventually(node).Should(ContainElement(bridge1))
 				By(fmt.Sprintf("Check that %s is rolled back again", bridge1))
 				interfacesNameForNodeEventually(node).ShouldNot(ContainElement(bridge1))
 			}
 		})
 	})
+	Context("when connectivity to default gw is lost after state configuration", func() {
+		BeforeEach(func() {
+			By("Configure a invalid default gw")
+			for _, node := range nodes {
+				var address string
+				Eventually(func() string {
+					address = ipv4Address(node, "eth0")
+					return address
+				}, ReadTimeout, ReadInterval).ShouldNot(BeEmpty())
+				updateDesiredStateAtNode(node, badDefaultGw(address, "eth0"))
+			}
+		})
+		AfterEach(func() {
+			By("Clean up desired state")
+			resetDesiredStateForNodes()
+		})
+		It("should rollback to a good gw configuration", func() {
+			for _, node := range nodes {
+				By("Check that desiredState is applied")
+				Eventually(func() bool {
+					return dhcpFlag(node, "eth0")
+				}, ReadTimeout, ReadInterval).Should(BeFalse())
+
+				By("Check that eth0 is rolled back")
+				Eventually(func() bool {
+					return dhcpFlag(node, "eth0")
+				}, ReadTimeout, ReadInterval).Should(BeTrue())
+			}
+		})
+	})
 })

test/e2e/utils.go

@@ -180,7 +180,7 @@ func updateDesiredStateAtNode(node string, desiredState nmstatev1alpha1.State) {
 		}
 		state.Spec.DesiredState = desiredState
 		return framework.Global.Client.Update(context.TODO(), &state)
-	}, ReadTimeout, ReadInterval).ShouldNot(HaveOccurred())
+	}, ReadTimeout, ReadInterval).ShouldNot(HaveOccurred(), string(desiredState))
 }
 
 func updateDesiredState(desiredState nmstatev1alpha1.State) {
@@ -315,7 +315,6 @@ func deleteConnectionAtNodes(name string) []error {
 }
 
 func interfaces(state nmstatev1alpha1.State) []interface{} {
-	By("unmarshal state yaml into unstructured golang")
 	var stateUnstructured map[string]interface{}
 	err := yaml.Unmarshal(state, &stateUnstructured)
 	Expect(err).ToNot(HaveOccurred(), "Should parse correctly yaml: %s", state)
@@ -475,3 +474,21 @@ func nextBond() string {
 	bridgeCounter++
 	return fmt.Sprintf("bond%d", bondConunter)
 }
+
+func currentStateJSON(node string) []byte {
+	key := types.NamespacedName{Name: node}
+	currentState := nodeNetworkState(key).Status.CurrentState
+	currentStateJson, err := yaml.YAMLToJSON([]byte(currentState))
+	ExpectWithOffset(1, err).ToNot(HaveOccurred())
+	return currentStateJson
+}
+
+func dhcpFlag(node string, name string) bool {
+	path := fmt.Sprintf("interfaces.#(name==\"%s\").ipv4.dhcp", name)
+	return gjson.ParseBytes(currentStateJSON(node)).Get(path).Bool()
+}
+
+func ipv4Address(node string, name string) string {
+	path := fmt.Sprintf("interfaces.#(name==\"%s\").ipv4.address.0.ip", name)
+	return gjson.ParseBytes(currentStateJSON(node)).Get(path).String()
+}