Merge pull request #406 from tariq1890/pretty-cni-config

Pretty print CNI config when rendering NetAttachDefs

controllers/helper.go

@@ -17,6 +17,8 @@ limitations under the License.
 package controllers
 
 import (
+	"bytes"
+	"encoding/json"
 	"os"
 	"strings"
 
@@ -38,3 +40,11 @@ func GetImagePullSecrets() []string {
 		return []string{}
 	}
 }
+
+func formatJSON(str string) (string, error) {
+	var prettyJSON bytes.Buffer
+	if err := json.Indent(&prettyJSON, []byte(str), "", "    "); err != nil {
+		return "", err
+	}
+	return prettyJSON.String(), nil
+}

controllers/sriovibnetwork_controller.go

@@ -117,6 +117,12 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if err != nil {
 		return reconcile.Result{}, err
 	}
+	// format CNI config json in CR for easier readability
+	netAttDef.Spec.Config, err = formatJSON(netAttDef.Spec.Config)
+	if err != nil {
+		reqLogger.Error(err, "Couldn't process rendered NetworkAttachmentDefinition config", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name)
+		return reconcile.Result{}, err
+	}
 	if lnns, ok := instance.GetAnnotations()[sriovnetworkv1.LASTNETWORKNAMESPACE]; ok && netAttDef.GetNamespace() != lnns {
 		err = r.Delete(ctx, &netattdefv1.NetworkAttachmentDefinition{
 			ObjectMeta: metav1.ObjectMeta{

controllers/sriovibnetwork_controller_test.go

@@ -47,7 +47,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 		DescribeTable("should be possible to create/delete net-att-def",
 			func(cr sriovnetworkv1.SriovIBNetwork) {
 				var err error
-				expect := util.GenerateExpectedIBNetConfig(&cr)
+				expect := generateExpectedIBNetConfig(&cr)
 
 				By("Create the SriovIBNetwork Custom Resource")
 				// get global framework variables
@@ -104,7 +104,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 					Expect(k8sClient.Delete(goctx.TODO(), &old)).To(Succeed())
 				}()
 				found := &sriovnetworkv1.SriovIBNetwork{}
-				expect := util.GenerateExpectedIBNetConfig(&new)
+				expect := generateExpectedIBNetConfig(&new)
 
 				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 					// Retrieve the latest version of SriovIBNetwork before attempting update
@@ -161,7 +161,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 				},
 			}
 			var err error
-			expect := util.GenerateExpectedIBNetConfig(&cr)
+			expect := generateExpectedIBNetConfig(&cr)
 
 			err = k8sClient.Create(goctx.TODO(), &cr)
 			Expect(err).NotTo(HaveOccurred())
@@ -190,3 +190,29 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 		})
 	})
 })
+
+func generateExpectedIBNetConfig(cr *sriovnetworkv1.SriovIBNetwork) string {
+	ipam := emptyCurls
+	state := getLinkState(cr.Spec.LinkState)
+
+	if cr.Spec.IPAM != "" {
+		ipam = cr.Spec.IPAM
+	}
+	configStr, err := formatJSON(fmt.Sprintf(`{ "cniVersion":"0.3.1", "name":"%s","type":"ib-sriov",%s"ipam":%s }`, cr.GetName(), state, ipam))
+	if err != nil {
+		panic(err)
+	}
+	return configStr
+}
+
+func getLinkState(state string) string {
+	st := ""
+	if state == sriovnetworkv1.SriovCniStateAuto {
+		st = `"link_state":"auto",`
+	} else if state == sriovnetworkv1.SriovCniStateEnable {
+		st = `"link_state":"enable",`
+	} else if state == sriovnetworkv1.SriovCniStateDisable {
+		st = `"link_state":"disable",`
+	}
+	return st
+}

controllers/sriovnetwork_controller.go

@@ -118,6 +118,12 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	if err != nil {
 		return reconcile.Result{}, err
 	}
+	// format CNI config json in CR for easier readability
+	netAttDef.Spec.Config, err = formatJSON(netAttDef.Spec.Config)
+	if err != nil {
+		reqLogger.Error(err, "Couldn't process rendered NetworkAttachmentDefinition config", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name)
+		return reconcile.Result{}, err
+	}
 	if lnns, ok := instance.GetAnnotations()[sriovnetworkv1.LASTNETWORKNAMESPACE]; ok && netAttDef.GetNamespace() != lnns {
 		err = r.Delete(ctx, &netattdefv1.NetworkAttachmentDefinition{
 			ObjectMeta: metav1.ObjectMeta{

controllers/sriovnetwork_controller_test.go

@@ -20,7 +20,10 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util"
 )
 
-const on = "on"
+const (
+	on         = "on"
+	emptyCurls = "{}"
+)
 
 var _ = Describe("SriovNetwork Controller", func() {
 
@@ -55,7 +58,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 		DescribeTable("should be possible to create/delete net-att-def",
 			func(cr sriovnetworkv1.SriovNetwork) {
 				var err error
-				expect := util.GenerateExpectedNetConfig(&cr)
+				expect := generateExpectedNetConfig(&cr)
 
 				By("Create the SriovNetwork Custom Resource")
 				// get global framework variables
@@ -123,7 +126,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 				}()
 				Expect(err).NotTo(HaveOccurred())
 				found := &sriovnetworkv1.SriovNetwork{}
-				expect := util.GenerateExpectedNetConfig(&new)
+				expect := generateExpectedNetConfig(&new)
 
 				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 					// Retrieve the latest version of SriovNetwork before attempting update
@@ -183,7 +186,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 					},
 				}
 				var err error
-				expect := util.GenerateExpectedNetConfig(&cr)
+				expect := generateExpectedNetConfig(&cr)
 
 				err = k8sClient.Create(goctx.TODO(), &cr)
 				Expect(err).NotTo(HaveOccurred())
@@ -213,3 +216,34 @@ var _ = Describe("SriovNetwork Controller", func() {
 		})
 	})
 })
+
+func generateExpectedNetConfig(cr *sriovnetworkv1.SriovNetwork) string {
+	spoofchk := ""
+	trust := ""
+	ipam := emptyCurls
+
+	if cr.Spec.Trust == sriovnetworkv1.SriovCniStateOn {
+		trust = `"trust":"on",`
+	} else if cr.Spec.Trust == sriovnetworkv1.SriovCniStateOff {
+		trust = `"trust":"off",`
+	}
+
+	if cr.Spec.SpoofChk == sriovnetworkv1.SriovCniStateOn {
+		spoofchk = `"spoofchk":"on",`
+	} else if cr.Spec.SpoofChk == sriovnetworkv1.SriovCniStateOff {
+		spoofchk = `"spoofchk":"off",`
+	}
+
+	state := getLinkState(cr.Spec.LinkState)
+
+	if cr.Spec.IPAM != "" {
+		ipam = cr.Spec.IPAM
+	}
+	vlanQoS := cr.Spec.VlanQoS
+
+	configStr, err := formatJSON(fmt.Sprintf(`{ "cniVersion":"0.3.1", "name":"%s","type":"sriov","vlan":%d,%s%s%s"vlanQoS":%d,"ipam":%s }`, cr.GetName(), cr.Spec.Vlan, spoofchk, trust, state, vlanQoS, ipam))
+	if err != nil {
+		panic(err)
+	}
+	return configStr
+}

test/util/util.go

@@ -32,8 +32,6 @@ var (
 	CleanupTimeout       = time.Second * 5
 )
 
-const emptyCurls = "{}"
-
 func WaitForSriovNetworkNodeStateReady(nodeState *sriovnetworkv1.SriovNetworkNodeState, client client.Client, namespace, name string, retryInterval, timeout time.Duration) error {
 	time.Sleep(30 * time.Second)
 	err := wait.PollImmediate(retryInterval, timeout, func() (done bool, err error) {
@@ -145,45 +143,6 @@ func GenerateSriovNetworkCRs(namespace string, specs map[string]sriovnetworkv1.S
 	return crs
 }
 
-func GenerateExpectedNetConfig(cr *sriovnetworkv1.SriovNetwork) string {
-	spoofchk := ""
-	trust := ""
-	ipam := emptyCurls
-
-	if cr.Spec.Trust == sriovnetworkv1.SriovCniStateOn {
-		trust = `"trust":"on",`
-	} else if cr.Spec.Trust == sriovnetworkv1.SriovCniStateOff {
-		trust = `"trust":"off",`
-	}
-
-	if cr.Spec.SpoofChk == sriovnetworkv1.SriovCniStateOn {
-		spoofchk = `"spoofchk":"on",`
-	} else if cr.Spec.SpoofChk == sriovnetworkv1.SriovCniStateOff {
-		spoofchk = `"spoofchk":"off",`
-	}
-
-	state := getLinkState(cr.Spec.LinkState)
-
-	if cr.Spec.IPAM != "" {
-		ipam = cr.Spec.IPAM
-	}
-	vlanQoS := cr.Spec.VlanQoS
-
-	return fmt.Sprintf(`{ "cniVersion":"0.3.1", "name":"%s","type":"sriov","vlan":%d,%s%s%s"vlanQoS":%d,"ipam":%s }`, cr.GetName(), cr.Spec.Vlan, spoofchk, trust, state, vlanQoS, ipam)
-}
-
-func getLinkState(state string) string {
-	st := ""
-	if state == sriovnetworkv1.SriovCniStateAuto {
-		st = `"link_state":"auto",`
-	} else if state == sriovnetworkv1.SriovCniStateEnable {
-		st = `"link_state":"enable",`
-	} else if state == sriovnetworkv1.SriovCniStateDisable {
-		st = `"link_state":"disable",`
-	}
-	return st
-}
-
 func GenerateSriovIBNetworkCRs(namespace string, specs map[string]sriovnetworkv1.SriovIBNetworkSpec) map[string]sriovnetworkv1.SriovIBNetwork {
 	crs := make(map[string]sriovnetworkv1.SriovIBNetwork)
 
@@ -203,16 +162,6 @@ func GenerateSriovIBNetworkCRs(namespace string, specs map[string]sriovnetworkv1
 	return crs
 }
 
-func GenerateExpectedIBNetConfig(cr *sriovnetworkv1.SriovIBNetwork) string {
-	ipam := emptyCurls
-	state := getLinkState(cr.Spec.LinkState)
-
-	if cr.Spec.IPAM != "" {
-		ipam = cr.Spec.IPAM
-	}
-	return fmt.Sprintf(`{ "cniVersion":"0.3.1", "name":"%s","type":"ib-sriov",%s"ipam":%s }`, cr.GetName(), state, ipam)
-}
-
 func ValidateDevicePluginConfig(nps []*sriovnetworkv1.SriovNetworkNodePolicy, rawConfig string) error {
 	rcl := dptypes.ResourceConfList{}