From d508dcecae7174133ee253cc068fe7fe11b8d5fe Mon Sep 17 00:00:00 2001 From: Gareth McFarlane Date: Mon, 15 Sep 2025 05:47:37 +0000 Subject: [PATCH 1/5] [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. --- topo/node/juniper/juniper.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/topo/node/juniper/juniper.go b/topo/node/juniper/juniper.go index 9fffe672..270e5341 100644 --- a/topo/node/juniper/juniper.go +++ b/topo/node/juniper/juniper.go @@ -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 From cd8635f613078830597ccda8f5441e5d56ba8f18 Mon Sep 17 00:00:00 2001 From: Gareth McFarlane Date: Mon, 15 Sep 2025 06:02:58 +0000 Subject: [PATCH 2/5] Format Go --- topo/node/juniper/juniper.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/topo/node/juniper/juniper.go b/topo/node/juniper/juniper.go index 270e5341..415180de 100644 --- a/topo/node/juniper/juniper.go +++ b/topo/node/juniper/juniper.go @@ -207,16 +207,16 @@ func (n *Node) GRPCConfig() []string { } // 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 + // 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...) + 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 + return conf } // Waits and retries until CLI config mode is up and config is applied From 05980945344d64e8737f35ac71d359d89accfe41 Mon Sep 17 00:00:00 2001 From: Gareth McFarlane Date: Tue, 16 Sep 2025 01:57:14 +0000 Subject: [PATCH 3/5] Add unit tests --- topo/node/juniper/juniper_test.go | 81 +++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/topo/node/juniper/juniper_test.go b/topo/node/juniper/juniper_test.go index 4184fae2..fa77af2c 100644 --- a/topo/node/juniper/juniper_test.go +++ b/topo/node/juniper/juniper_test.go @@ -218,6 +218,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{ From 107d1891e66091d1d8fb0d903ffd0c3e96fa39fe Mon Sep 17 00:00:00 2001 From: Gareth McFarlane Date: Wed, 17 Sep 2025 01:20:12 +0000 Subject: [PATCH 4/5] Handle errTimeoutError and increase config push deadline by 5 minutes. --- topo/node/juniper/juniper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/topo/node/juniper/juniper.go b/topo/node/juniper/juniper.go index 415180de..af5e55c7 100644 --- a/topo/node/juniper/juniper.go +++ b/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 @@ -226,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) From f348f602e6b35ed5b548dedd826a6424ed950fa0 Mon Sep 17 00:00:00 2001 From: Gareth McFarlane Date: Wed, 17 Sep 2025 01:46:48 +0000 Subject: [PATCH 5/5] Fix tests --- topo/node/juniper/juniper_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/topo/node/juniper/juniper_test.go b/topo/node/juniper/juniper_test.go index fa77af2c..a7039b71 100644 --- a/topo/node/juniper/juniper_test.go +++ b/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