[Juniper] Only set the legacy gRPC services config if the specific node label isn't set. (#647)

* [Juniper] Only set the legacy gRPC services config if the specific node label isn't set.

These config lines are now not required in D47 and will cause future config commits to fail if combined with the newer gRPC services configuration.

* Format Go

* Add unit tests

* Handle errTimeoutError and increase config push deadline by 5 minutes.

* Fix tests

Author: gmacf

topo/node/juniper/juniper.go

@@ -33,11 +33,11 @@ var (
 	// For committing a very large config
 	scrapliOperationTimeout = 300 * time.Second
 	// Wait for PKI cert infra
-	certGenTimeout = 10 * time.Minute
+	certGenTimeout = 15 * time.Minute
 	// Time between polls
 	certGenRetrySleep = 30 * time.Second
 	// Wait for config mode
-	configModeTimeout = 10 * time.Minute
+	configModeTimeout = 15 * time.Minute
 	// Time between polls - config mode
 	configModeRetrySleep = 30 * time.Second
 	// Default gRPC port
@@ -196,9 +196,7 @@ func (n *Node) GRPCConfig() []string {
 	}
 	log.Infof("gNMI Port %d", port)
 	portConfig := fmt.Sprintf("set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config port %d", port)
-	return []string{
-		"set system services extension-service request-response grpc ssl hot-reloading",
-		"set system services extension-service request-response grpc ssl use-pki",
+	conf := []string{
 		"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config services GNMI",
 		"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config enable true",
 		portConfig,
@@ -207,6 +205,18 @@ func (n *Node) GRPCConfig() []string {
 		"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config listen-addresses 0.0.0.0",
 		"commit",
 	}
+	// In newer Juniper releases such as D47, hot reloading and PKI support is enabled by default. On these systems, the legacy
+	// syntax below is mutually exclusive with the new gRPC service config. Attempting to configure both will cause the config
+	// commit to fail. Therefore, if configuring gRPC services via CLI on a release from D47 onwards, a KNE Node label of
+	// `legacy_grpc_server_config`` should be set to `disabled.`
+	if n.GetProto().GetLabels()["legacy_grpc_server_config"] != "disabled" {
+		legacyConf := []string{
+			"set system services extension-service request-response grpc ssl hot-reloading",
+			"set system services extension-service request-response grpc ssl use-pki",
+		}
+		conf = append(legacyConf, conf...)
+	}
+	return conf
 }
 
 // Waits and retries until CLI config mode is up and config is applied
@@ -216,7 +226,7 @@ func (n *Node) waitConfigInfraReadyAndPushConfigs(configs []string) error {
 	for time.Since(start) < configModeTimeout {
 		multiresp, err := n.cliConn.SendConfigs(configs)
 		if err != nil {
-			if strings.Contains(err.Error(), "errPrivilegeError") {
+			if strings.Contains(err.Error(), "errPrivilegeError") || strings.Contains(err.Error(), "errTimeoutError") {
 				log.Infof("Config mode not ready. Retrying in %v. Node %s, Resp %v", configModeRetrySleep, n.Name(), err)
 			} else {
 				return fmt.Errorf("failed pushing configs: %v", err)

topo/node/juniper/juniper_test.go

@@ -142,6 +142,18 @@ func TestGenerateSelfSigned(t *testing.T) {
 	}()
 	configModeRetrySleep = time.Millisecond
 
+	origCertGenTimeout := certGenTimeout
+	defer func() {
+		certGenTimeout = origCertGenTimeout
+	}()
+	certGenTimeout = time.Second * 10
+
+	origConfigModeTimeout := configModeTimeout
+	defer func() {
+		configModeTimeout = origConfigModeTimeout
+	}()
+	configModeTimeout = time.Second * 10
+
 	tests := []struct {
 		desc     string
 		wantErr  bool
@@ -218,6 +230,87 @@ func TestGenerateSelfSigned(t *testing.T) {
 	}
 }
 
+func TestGRPCConfig(t *testing.T) {
+	tests := []struct {
+		desc string
+		ni   *node.Impl
+		want []string
+	}{
+		{
+			desc: "legacy grpc server config",
+			ni: &node.Impl{
+				KubeClient: fake.NewSimpleClientset(),
+				Namespace:  "test",
+				Proto: &tpb.Node{
+					Name:   "pod1",
+					Vendor: tpb.Vendor_JUNIPER,
+					Config: &tpb.Config{
+						ConfigFile: "foo",
+						ConfigPath: "/",
+						ConfigData: &tpb.Config_Data{
+							Data: []byte("config file data"),
+						},
+					},
+				},
+			},
+			want: []string{
+				"set system services extension-service request-response grpc ssl hot-reloading",
+				"set system services extension-service request-response grpc ssl use-pki",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config services GNMI",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config enable true",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config port 32767",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config transport-security true",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config certificate-id grpc-server-cert",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config listen-addresses 0.0.0.0",
+				"commit",
+			},
+		},
+		{
+			desc: "new grpc server config",
+			ni: &node.Impl{
+				KubeClient: fake.NewSimpleClientset(),
+				Namespace:  "test",
+				Proto: &tpb.Node{
+					Name:   "pod1",
+					Vendor: tpb.Vendor_JUNIPER,
+					Config: &tpb.Config{
+						ConfigFile: "foo",
+						ConfigPath: "/",
+						ConfigData: &tpb.Config_Data{
+							Data: []byte("config file data"),
+						},
+					},
+					Labels: map[string]string{
+						"legacy_grpc_server_config": "disabled",
+					},
+				},
+			},
+			want: []string{
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config services GNMI",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config enable true",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config port 32767",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config transport-security true",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config certificate-id grpc-server-cert",
+				"set openconfig-system:system openconfig-system-grpc:grpc-servers grpc-server grpc-server config listen-addresses 0.0.0.0",
+				"commit",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			nImpl, err := New(tt.ni)
+			if err != nil {
+				t.Fatalf("failed creating kne juniper node")
+			}
+			n, _ := nImpl.(*Node)
+			got := n.GRPCConfig()
+			if diff := cmp.Diff(tt.want, got); diff != "" {
+				t.Errorf("GRPCConfig() returned unexpected diff (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestConfigPush(t *testing.T) {
 	ki := fake.NewSimpleClientset(&corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{