Added Available and Failing conditions (#154)

This patch adds conditions reporting on NodeNetworkState informing user about the result of last executed `nmstatectl set`.

pkg/apis/nmstate/v1alpha1/nodenetworkstate_types.go

@@ -48,7 +48,6 @@ const (
 	NodeNetworkStateConditionFailing     NodeNetworkStateConditionType = "Failing"
 	NodeNetworkStateConditionProgressing NodeNetworkStateConditionType = "Progressing"
 	NodeNetworkStateConditionAvailable   NodeNetworkStateConditionType = "Available"
-	NodeNetworkStateConditionInitialized NodeNetworkStateConditionType = "Initialized"
 )
 
 // +genclient

pkg/controller/nodenetworkstateconfiguration/nodenetworkstateconfiguration_controller.go

@@ -6,8 +6,11 @@ import (
 	"reflect"
 
 	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -18,6 +21,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/nmstate/kubernetes-nmstate/pkg/controller/conditions"
 	nmstate "github.com/nmstate/kubernetes-nmstate/pkg/helper"
 )
 
@@ -101,6 +105,63 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 	return nil
 }
 
+func setCondtionFailed(instance *nmstatev1alpha1.NodeNetworkState, message string) {
+	conditions.SetCondition(
+		instance,
+		nmstatev1alpha1.NodeNetworkStateConditionFailing,
+		corev1.ConditionTrue,
+		"FailedToConfigure",
+		message,
+	)
+	conditions.SetCondition(
+		instance,
+		nmstatev1alpha1.NodeNetworkStateConditionAvailable,
+		corev1.ConditionFalse,
+		"FailedToConfigure",
+		message,
+	)
+}
+
+func setCondtionSuccess(instance *nmstatev1alpha1.NodeNetworkState, message string) {
+	conditions.SetCondition(
+		instance,
+		nmstatev1alpha1.NodeNetworkStateConditionAvailable,
+		corev1.ConditionTrue,
+		"SuccessfullyConfigured",
+		message,
+	)
+	conditions.SetCondition(
+		instance,
+		nmstatev1alpha1.NodeNetworkStateConditionFailing,
+		corev1.ConditionFalse,
+		"SuccessfullyConfigured",
+		"",
+	)
+}
+
+func (r *ReconcileNodeNetworkStateConfiguration) setCondition(
+	available bool,
+	message string,
+	stateName types.NamespacedName,
+) error {
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		instance := &nmstatev1alpha1.NodeNetworkState{}
+		err := r.client.Get(context.TODO(), stateName, instance)
+		if err != nil {
+			return err
+		}
+
+		if available {
+			setCondtionSuccess(instance, message)
+		} else {
+			setCondtionFailed(instance, message)
+		}
+
+		err = r.client.Status().Update(context.TODO(), instance)
+		return err
+	})
+}
+
 // blank assignment to verify that ReconcileNodeNetworkStateConfiguration implements reconcile.Reconciler
 var _ reconcile.Reconciler = &ReconcileNodeNetworkStateConfiguration{}
 
@@ -139,9 +200,21 @@ func (r *ReconcileNodeNetworkStateConfiguration) Reconcile(request reconcile.Req
 
 	nmstateOutput, err := nmstate.ApplyDesiredState(instance)
 	if err != nil {
-		return reconcile.Result{}, fmt.Errorf("error reconciling nodenetworkstate configuration at desired state apply: %v", err)
+		errmsg := fmt.Errorf("error reconciling nodenetworkstate at desired state apply: %v", err)
+
+		retryErr := r.setCondition(false, errmsg.Error(), request.NamespacedName)
+		if retryErr != nil {
+			reqLogger.Error(retryErr, "Failing condition update failed while reporting error: %v", errmsg)
+		}
+
+		return reconcile.Result{}, errmsg
 	}
 	reqLogger.Info("nmstate", "output", nmstateOutput)
 
+	err = r.setCondition(true, "successfully reconciled NodeNetworkState", request.NamespacedName)
+	if err != nil {
+		reqLogger.Error(err, "Success condition update failed while reporting success: %v", err)
+	}
+
 	return reconcile.Result{}, nil
 }

pkg/helper/client.go

@@ -99,9 +99,7 @@ func UpdateCurrentState(client client.Client, nodeNetworkState *nmstatev1alpha1.
 	}
 
 	// Let's update status with current network config from nmstatectl
-	nodeNetworkState.Status = nmstatev1alpha1.NodeNetworkStateStatus{
-		CurrentState: nmstatev1alpha1.State(currentState),
-	}
+	nodeNetworkState.Status.CurrentState = nmstatev1alpha1.State(currentState)
 
 	err = client.Status().Update(context.Background(), nodeNetworkState)
 	if err != nil {

test/e2e/nns_conditions_test.go

@@ -0,0 +1,70 @@
+package e2e
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	corev1 "k8s.io/api/core/v1"
+
+	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
+)
+
+var _ = Describe("NodeNetworkStateCondition", func() {
+	var (
+		br1Up = nmstatev1alpha1.State(`interfaces:
+  - name: br1
+    type: linux-bridge
+    state: up
+    bridge:
+      port:
+        - name: eth1
+`)
+		br1Absent = nmstatev1alpha1.State(`interfaces:
+  - name: br1
+    type: linux-bridge
+    state: absent
+`)
+		invalidConfig = nmstatev1alpha1.State(`interfaces:
+  - name: br1
+    type: linux-bridge
+    state: invalid_state
+`)
+	)
+	Context("when applying valid config", func() {
+		BeforeEach(func() {
+			updateDesiredState(br1Up)
+		})
+		AfterEach(func() {
+			updateDesiredState(br1Absent)
+			for _, node := range nodes {
+				interfacesNameForNode(node).ShouldNot(ContainElement("br1"))
+			}
+		})
+		It("should have Available ConditionType set to true", func() {
+			for _, node := range nodes {
+				checkCondition(node, nmstatev1alpha1.NodeNetworkStateConditionAvailable).Should(
+					Equal(corev1.ConditionTrue),
+				)
+				checkCondition(node, nmstatev1alpha1.NodeNetworkStateConditionFailing).Should(
+					Equal(corev1.ConditionFalse),
+				)
+			}
+		})
+	})
+
+	Context("when applying invalid configuration", func() {
+		BeforeEach(func() {
+			updateDesiredState(invalidConfig)
+		})
+		It("should have Failing ConditionType set to true", func() {
+			for _, node := range nodes {
+				checkCondition(node, nmstatev1alpha1.NodeNetworkStateConditionFailing).Should(
+					Equal(corev1.ConditionTrue),
+				)
+				checkCondition(node, nmstatev1alpha1.NodeNetworkStateConditionAvailable).Should(
+					Equal(corev1.ConditionFalse),
+				)
+			}
+		})
+	})
+})

test/e2e/utils.go

@@ -27,6 +27,7 @@ import (
 	dynclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
+	"github.com/nmstate/kubernetes-nmstate/pkg/controller/conditions"
 )
 
 const ReadTimeout = 120 * time.Second
@@ -289,6 +290,18 @@ func currentState(namespace string, node string, currentStateYaml *nmstatev1alph
 	}, ReadTimeout, ReadInterval)
 }
 
+func checkCondition(node string, conditionType nmstatev1alpha1.NodeNetworkStateConditionType) AsyncAssertion {
+	key := types.NamespacedName{Name: node}
+	return Eventually(func() corev1.ConditionStatus {
+		state := nodeNetworkState(key)
+		condition := conditions.Condition(&state, conditionType)
+		if condition == nil {
+			return corev1.ConditionUnknown
+		}
+		return condition.Status
+	}, ReadTimeout, ReadInterval)
+}
+
 func desiredState(namespace string, node string, desiredStateYaml *nmstatev1alpha1.State) AsyncAssertion {
 	key := types.NamespacedName{Namespace: namespace, Name: node}
 	return Eventually(func() nmstatev1alpha1.State {
@@ -374,3 +387,11 @@ func bridgeDescription(node string, bridgeName string) AsyncAssertion {
 		return run(node, "sudo", "ip", "-d", "link", "show", "type", "bridge", bridgeName)
 	}, ReadTimeout, ReadTimeout)
 }
+
+func conditionsToYaml(conditions []nmstatev1alpha1.NodeNetworkStateCondition) string {
+	manifest, err := yaml.Marshal(conditions)
+	if err != nil {
+		panic(err)
+	}
+	return string(manifest)
+}