diff --git a/topo/node/arista/arista.go b/topo/node/arista/arista.go index fea19833..b5e46c2d 100644 --- a/topo/node/arista/arista.go +++ b/topo/node/arista/arista.go @@ -177,7 +177,12 @@ func (n *Node) Status(ctx context.Context) (node.Status, error) { case corev1.PodPending: return node.StatusPending, nil case corev1.PodRunning: - return node.StatusRunning, nil + for _, cond := range p[0].Status.Conditions { + if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue { + return node.StatusRunning, nil + } + } + return node.StatusPending, nil case corev1.PodFailed: return node.StatusFailed, nil default: diff --git a/topo/node/arista/arista_test.go b/topo/node/arista/arista_test.go index 3a99ade8..1294ea2e 100644 --- a/topo/node/arista/arista_test.go +++ b/topo/node/arista/arista_test.go @@ -558,11 +558,12 @@ func TestResetCfg(t *testing.T) { func TestStatus(t *testing.T) { tests := []struct { - desc string - cantWatch bool - noPodYet bool - phase corev1.PodPhase - status node.Status + desc string + cantWatch bool + noPodYet bool + phase corev1.PodPhase + conditions []corev1.PodCondition + status node.Status }{ { desc: "can't watch pod status", @@ -581,10 +582,24 @@ func TestStatus(t *testing.T) { status: node.StatusPending, }, { - desc: "pod running", + desc: "pod running not ready", phase: corev1.PodRunning, + status: node.StatusPending, + }, + { + desc: "pod running and ready", + phase: corev1.PodRunning, + conditions: []corev1.PodCondition{{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }}, status: node.StatusRunning, }, + { + desc: "pod failed", + phase: corev1.PodFailed, + status: node.StatusFailed, + }, } ctx := context.Background() @@ -602,7 +617,8 @@ func TestStatus(t *testing.T) { Namespace: ns, }, Status: corev1.PodStatus{ - Phase: tt.phase, + Phase: tt.phase, + Conditions: tt.conditions, }, }) } diff --git a/topo/node/ciena/ciena.go b/topo/node/ciena/ciena.go index 174b86b1..4757814e 100644 --- a/topo/node/ciena/ciena.go +++ b/topo/node/ciena/ciena.go @@ -238,7 +238,8 @@ func (n *Node) CreatePod(ctx context.Context) error { SecurityContext: &corev1.SecurityContext{ Privileged: ptr.To(true), }, - VolumeMounts: extraMounts, + ReadinessProbe: node.ServiceReadinessProbe(pb), + VolumeMounts: extraMounts, }}, Volumes: extraVolumes, TerminationGracePeriodSeconds: ptr.To[int64](0), diff --git a/topo/node/ciena/ciena_test.go b/topo/node/ciena/ciena_test.go index 6789e6cf..30eb6fa7 100644 --- a/topo/node/ciena/ciena_test.go +++ b/topo/node/ciena/ciena_test.go @@ -25,7 +25,9 @@ import ( "github.com/openconfig/kne/topo/node" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" kfake "k8s.io/client-go/kubernetes/fake" ) @@ -305,6 +307,12 @@ func TestNode_CreatePod_EquipmentFile(t *testing.T) { Image: "vrnetlab/ciena_waverouter:config", VendorData: vendorData, }, + Services: map[uint32]*tpb.Service{ + 22: { + Names: []string{"ssh"}, + Inside: 22, + }, + }, } n := &Node{ @@ -337,6 +345,19 @@ func TestNode_CreatePod_EquipmentFile(t *testing.T) { if ctr.Image != "vrnetlab/ciena_waverouter:config" { t.Errorf("container image: got %q, want %q", ctr.Image, "vrnetlab/ciena_waverouter:config") } + wantProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + } + if diff := cmp.Diff(wantProbe, ctr.ReadinessProbe); diff != "" { + t.Errorf("container readiness probe mismatch (-want +got):\n%s", diff) + } foundMount := false for _, m := range ctr.VolumeMounts { if m.Name == "equipment-file" && m.MountPath == "/equipment/setup.json" && m.SubPath == "setup.json" && m.ReadOnly { diff --git a/topo/node/cisco/cisco.go b/topo/node/cisco/cisco.go index 76b876bc..25767e63 100644 --- a/topo/node/cisco/cisco.go +++ b/topo/node/cisco/cisco.go @@ -114,7 +114,10 @@ var ( } ) -var podIsUpRegex = regexp.MustCompile(`Router up`) +var ( + podIsUpRegex = regexp.MustCompile(`Router up|Vxr up`) + podIsFailedRegex = regexp.MustCompile(`Router failed to come up|FATAL sim:|LoginTimeoutError`) +) func New(nodeImpl *node.Impl) (node.Node, error) { if nodeImpl == nil { @@ -180,6 +183,7 @@ func (n *Node) Create(ctx context.Context) error { tty = true stdin = true } + readinessProbe := node.ServiceReadinessProbe(pb) pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: n.Name(), @@ -200,6 +204,7 @@ func (n *Node) Create(ctx context.Context) error { Resources: node.ToResourceRequirements(pb.Constraints), ImagePullPolicy: "IfNotPresent", SecurityContext: secContext, + ReadinessProbe: readinessProbe, VolumeMounts: []corev1.VolumeMount{{ Name: fmt.Sprintf("%s-run-mount", pb.Name), ReadOnly: false, @@ -610,7 +615,8 @@ func defaults(pb *tpb.Node) (*tpb.Node, error) { } // Status returns the current node state. -// For 8000e nodes it checks the logs and return running if log contains "Router up" +// For 8000e nodes it checks the logs and return running if log contains "Router up" and pod is ready. +// For XRD nodes it returns running if pod is ready. func (n *Node) Status(ctx context.Context) (node.Status, error) { p, err := n.Pods(ctx) if err != nil { @@ -630,36 +636,57 @@ func (n *Node) Status(ctx context.Context) (node.Status, error) { pb := n.Proto if pb.GetModel() != ModelXRD { req := n.KubeClient.CoreV1().Pods(p[0].Namespace).GetLogs(p[0].Name, &corev1.PodLogOptions{}) - if !isNode8000eUp(ctx, req) { + logBytes, err := getPodLogs(ctx, req) + if err != nil { log.V(2).Infof("Cisco %s node %s status is %v", n.Proto.Model, n.Name(), node.StatusPending) return node.StatusPending, nil } - } - for _, cond := range p[0].Status.Conditions { - if cond.Type == corev1.PodReady && cond.Status != corev1.ConditionTrue { + if isNode8000eFailed(logBytes) { + log.Warningf("Cisco %s node %s failed to boot", n.Proto.Model, n.Name()) + return node.StatusFailed, fmt.Errorf("cisco %s node %s failed to boot", n.Proto.Model, n.Name()) + } + if !isNode8000eUp(logBytes) { log.V(2).Infof("Cisco %s node %s status is %v", n.Proto.Model, n.Name(), node.StatusPending) return node.StatusPending, nil } } - log.Infof("Cisco %s node %s status is %v ", n.Proto.Model, n.Name(), node.StatusRunning) - return node.StatusRunning, nil + for _, cond := range p[0].Status.Conditions { + if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue { + log.Infof("Cisco %s node %s status is %v ", n.Proto.Model, n.Name(), node.StatusRunning) + return node.StatusRunning, nil + } + } + log.V(2).Infof("Cisco %s node %s status is %v", n.Proto.Model, n.Name(), node.StatusPending) + return node.StatusPending, nil default: return node.StatusUnknown, nil } } -func isNode8000eUp(ctx context.Context, req *rest.Request) bool { +func getPodLogs(ctx context.Context, req *rest.Request) ([]byte, error) { podLogs, err := req.Stream(ctx) if err != nil { - return false + return nil, err } - defer podLogs.Close() + defer func() { + if err := podLogs.Close(); err != nil { + log.V(2).Infof("Failed to close pod logs stream: %v", err) + } + }() buf := new(bytes.Buffer) - len, err := io.Copy(buf, podLogs) - if err != nil || len == 0 { - return false + n, err := io.Copy(buf, podLogs) + if err != nil || n == 0 { + return nil, fmt.Errorf("failed to read pod logs or empty log") } - return podIsUpRegex.Match(buf.Bytes()) + return buf.Bytes(), nil +} + +func isNode8000eUp(logBytes []byte) bool { + return podIsUpRegex.Match(logBytes) +} + +func isNode8000eFailed(logBytes []byte) bool { + return podIsFailedRegex.Match(logBytes) } // No op function to override default network on open function. diff --git a/topo/node/cisco/cisco_test.go b/topo/node/cisco/cisco_test.go index 609011ab..3e395dbd 100644 --- a/topo/node/cisco/cisco_test.go +++ b/topo/node/cisco/cisco_test.go @@ -37,6 +37,7 @@ import ( "google.golang.org/protobuf/testing/protocmp" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" ) @@ -923,52 +924,214 @@ var ( } ) +func TestIsNode8000eFailed(t *testing.T) { + tests := []struct { + desc string + log string + want bool + }{ + { + desc: "Router failed to come up", + log: "Error: Router failed to come up after timeout", + want: true, + }, + { + desc: "FATAL sim:", + log: "2023-01-01 FATAL sim: initialization aborted", + want: true, + }, + { + desc: "LoginTimeoutError", + log: "Encountered LoginTimeoutError during boot", + want: true, + }, + { + desc: "Normal log with Router up", + log: "Router up and running", + want: false, + }, + { + desc: "Empty log", + log: "", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + if got := isNode8000eFailed([]byte(tt.log)); got != tt.want { + t.Errorf("isNode8000eFailed(%q) = %v, want %v", tt.log, got, tt.want) + } + }) + } +} + +func TestIsNode8000eUp(t *testing.T) { + tests := []struct { + desc string + log string + want bool + }{ + { + desc: "Router up", + log: "System initialized: Router up", + want: true, + }, + { + desc: "Vxr up", + log: "Line card online: Vxr up", + want: true, + }, + { + desc: "Booting log", + log: "Booting kernel...", + want: false, + }, + { + desc: "Empty log", + log: "", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + if got := isNode8000eUp([]byte(tt.log)); got != tt.want { + t.Errorf("isNode8000eUp(%q) = %v, want %v", tt.log, got, tt.want) + } + }) + } +} + func TestNodeStatus(t *testing.T) { tests := []struct { - desc string - status node.Status - ni *node.Impl - podLogErr bool + desc string + status node.Status + model string + phase corev1.PodPhase + conditions []corev1.PodCondition + podLogErr bool + podLogFail bool + wantErr bool }{ { - desc: "Status test for 8000e Node", + desc: "Status running for 8000e Node when Ready and Up", status: node.StatusRunning, - ni: node8000e, + model: "8201-32FH", + phase: corev1.PodRunning, + conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, }, { - desc: "Negative Status test for 8000e Node", + desc: "Status pending for 8000e Node when Up but Not Ready", + status: node.StatusPending, + model: "8201-32FH", + phase: corev1.PodRunning, + conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionFalse}, + }, + }, + { + desc: "Status pending for 8000e Node when log does not match up", status: node.StatusPending, - ni: node8000e, + model: "8201-32FH", + phase: corev1.PodRunning, podLogErr: true, + conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + }, + { + desc: "Status failed for 8000e Node when log contains failure pattern", + status: node.StatusFailed, + model: "8201-32FH", + phase: corev1.PodRunning, + podLogFail: true, + wantErr: true, }, { - desc: "Status test for XRD Node", + desc: "Status running for XRD Node when Ready", status: node.StatusRunning, - ni: nodeXRD, + model: ModelXRD, + phase: corev1.PodRunning, + conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, }, { - desc: "Status test for XRD Node, pod logs do not matter", - status: node.StatusRunning, - ni: nodeXRD, - podLogErr: true, + desc: "Status pending for XRD Node when Not Ready", + status: node.StatusPending, + model: ModelXRD, + phase: corev1.PodRunning, + conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionFalse}, + }, + }, + { + desc: "Status pending for XRD Node without Ready condition", + status: node.StatusPending, + model: ModelXRD, + phase: corev1.PodRunning, + }, + { + desc: "Status pending for PodPending phase", + status: node.StatusPending, + model: ModelXRD, + phase: corev1.PodPending, + }, + { + desc: "Status failed for PodFailed phase", + status: node.StatusFailed, + model: ModelXRD, + phase: corev1.PodFailed, }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { ctx := context.Background() - if !tt.podLogErr { + podName := "test-pod" + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: tt.phase, + Conditions: tt.conditions, + }, + } + ki := fake.NewSimpleClientset(pod) + ni := &node.Impl{ + KubeClient: ki, + Namespace: "test", + Proto: &tpb.Node{ + Name: podName, + Vendor: tpb.Vendor_CISCO, + Config: &tpb.Config{}, + Model: tt.model, + }, + } + if tt.podLogFail { + origPodIsFailedRegex := podIsFailedRegex + defer func() { + podIsFailedRegex = origPodIsFailedRegex + }() + podIsFailedRegex = regexp.MustCompile("fake log") + } else if !tt.podLogErr { origPodIsUpRegex := podIsUpRegex defer func() { podIsUpRegex = origPodIsUpRegex }() podIsUpRegex = regexp.MustCompile("fake log") // this is the expected log from a fake pod } - nImpl, _ := New(tt.ni) + nImpl, err := New(ni) + if err != nil { + t.Fatalf("New() failed: %v", err) + } n, _ := nImpl.(*Node) status, err := n.Status(ctx) - if err != nil { - t.Errorf("Error is not expected for Node Status") + if (err != nil) != tt.wantErr { + t.Errorf("node.Status() error = %v, wantErr %v", err, tt.wantErr) } if status != tt.status { t.Errorf("node.Status() = %v, want %v", status, tt.status) @@ -1203,21 +1366,22 @@ func TestCreate(t *testing.T) { if tt.configData != nil { cfg.ConfigData = &tpb.Config_Data{Data: tt.configData} } - n := &Node{ - Impl: &node.Impl{ - Namespace: "test", - KubeClient: ki, - Proto: &tpb.Node{ - Name: "node1", - Model: tt.model, - Config: cfg, - Interfaces: map[string]*tpb.Interface{ - "eth1": {Name: "GigabitEthernet0/0/0/0"}, - }, + nImpl, err := New(&node.Impl{ + Namespace: "test", + KubeClient: ki, + Proto: &tpb.Node{ + Name: "node1", + Model: tt.model, + Config: cfg, + Interfaces: map[string]*tpb.Interface{ + "eth1": {Name: "GigabitEthernet0/0/0/0"}, }, }, + }) + if err != nil { + t.Fatalf("New() unexpected error = %v", err) } - if err := n.Create(context.Background()); (err != nil) != tt.wantErr { + if err := nImpl.Create(context.Background()); (err != nil) != tt.wantErr { t.Fatalf("Create() unexpected error = %v, wantErr = %v", err, tt.wantErr) } pod, err := ki.CoreV1().Pods("test").Get(context.Background(), "node1", metav1.GetOptions{}) @@ -1240,6 +1404,19 @@ func TestCreate(t *testing.T) { if len(pod.Spec.Containers[0].VolumeMounts) != tt.wantMainMountsLen { t.Errorf("main container volume mounts len = %d, want %d", len(pod.Spec.Containers[0].VolumeMounts), tt.wantMainMountsLen) } + wantProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + } + if diff := cmp.Diff(wantProbe, pod.Spec.Containers[0].ReadinessProbe); diff != "" { + t.Errorf("container readiness probe mismatch (-want +got):\n%s", diff) + } for _, sub := range tt.wantInitScriptSub { if !strings.Contains(initC.Args[0], sub) { t.Errorf("init container script missing expected substring %q", sub) diff --git a/topo/node/juniper/juniper.go b/topo/node/juniper/juniper.go index 95ed2b51..4e1bf03a 100644 --- a/topo/node/juniper/juniper.go +++ b/topo/node/juniper/juniper.go @@ -546,6 +546,7 @@ func (n *Node) Create(ctx context.Context) error { Env: node.ToEnvVar(pb.Config.Env), Resources: node.ToResourceRequirements(pb.Constraints), ImagePullPolicy: "IfNotPresent", + ReadinessProbe: node.ServiceReadinessProbe(pb), SecurityContext: &corev1.SecurityContext{ Privileged: pointer.Bool(true), RunAsUser: pointer.Int64(0), diff --git a/topo/node/juniper/juniper_test.go b/topo/node/juniper/juniper_test.go index 88278fe0..67ca4cf7 100644 --- a/topo/node/juniper/juniper_test.go +++ b/topo/node/juniper/juniper_test.go @@ -28,6 +28,7 @@ import ( "google.golang.org/protobuf/testing/protocmp" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" ktest "k8s.io/client-go/testing" @@ -1041,6 +1042,19 @@ func TestCreate(t *testing.T) { if len(pod.Spec.Containers[0].VolumeMounts) != tt.wantMainMountsLen { t.Errorf("main container volume mounts len = %d, want %d", len(pod.Spec.Containers[0].VolumeMounts), tt.wantMainMountsLen) } + wantProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + } + if diff := cmp.Diff(wantProbe, pod.Spec.Containers[0].ReadinessProbe); diff != "" { + t.Errorf("container readiness probe mismatch (-want +got):\n%s", diff) + } for _, sub := range tt.wantInitScriptSub { if !strings.Contains(initC.Args[0], sub) { t.Errorf("init container script missing expected substring %q", sub) diff --git a/topo/node/node.go b/topo/node/node.go index 091956aa..01ea3766 100644 --- a/topo/node/node.go +++ b/topo/node/node.go @@ -7,6 +7,7 @@ import ( "math" "os" "path/filepath" + "sort" "strconv" "strings" "sync" @@ -229,6 +230,58 @@ func ToResourceRequirements(kv map[string]string) corev1.ResourceRequirements { return r } +// ServiceReadinessProbe returns a TCPSocket readiness probe for the node if a probeable service +// (e.g. ssh, gnmi) is defined in its service map. +func ServiceReadinessProbe(pb *tpb.Node) *corev1.Probe { + if pb == nil || len(pb.Services) == 0 { + return nil + } + keys := make([]uint32, 0, len(pb.Services)) + for k := range pb.Services { + keys = append(keys, k) + } + sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) + + // Prefer SSH first (typically available at boot across all NOS), then gNMI + preferred := []string{"ssh", "gnmi"} + for _, pref := range preferred { + for _, k := range keys { + svc := pb.Services[k] + if svc == nil { + continue + } + matched := strings.EqualFold(svc.Name, pref) + if !matched { + for _, name := range svc.Names { + if strings.EqualFold(name, pref) { + matched = true + break + } + } + } + if matched { + port := int(svc.Inside) + if port == 0 { + port = int(k) + } + if port > 0 { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(port), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + } + } + } + } + } + return nil +} + // Create will create the node in the k8s cluster with all services and config // maps. func (n *Impl) Create(ctx context.Context) error { @@ -440,6 +493,7 @@ func (n *Impl) CreatePod(ctx context.Context) error { SecurityContext: &corev1.SecurityContext{ Privileged: pointer.Bool(true), }, + ReadinessProbe: ServiceReadinessProbe(pb), }}, TerminationGracePeriodSeconds: pointer.Int64(0), NodeSelector: map[string]string{}, diff --git a/topo/node/node_test.go b/topo/node/node_test.go index 3119861c..9d4882c9 100644 --- a/topo/node/node_test.go +++ b/topo/node/node_test.go @@ -365,7 +365,7 @@ func TestValidateConstraints(t *testing.T) { constraintValues map[string]int }{ { - desc: "Invalid case - contraint value is greater than upper bound", + desc: "Invalid case - constraint value is greater than upper bound", node: &topopb.Node{ Name: "node1", HostConstraints: []*topopb.HostConstraint{ @@ -457,3 +457,173 @@ func TestValidateConstraints(t *testing.T) { }) } } + +func TestServiceReadinessProbe(t *testing.T) { + tests := []struct { + desc string + node *topopb.Node + want *corev1.Probe + }{ + { + desc: "nil node", + node: nil, + want: nil, + }, + { + desc: "no services", + node: &topopb.Node{}, + want: nil, + }, + { + desc: "ssh service", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 22: { + Name: "ssh", + Inside: 22, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + { + desc: "ssh service custom inside port", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 22: { + Name: "ssh", + Inside: 2222, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(2222), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + { + desc: "ssh service in names slice", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 22: { + Names: []string{"ssh", "cli"}, + Inside: 22, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + { + desc: "gnmi service only", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 9339: { + Name: "gnmi", + Inside: 57400, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(57400), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + { + desc: "both ssh and gnmi prefers ssh", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 22: { + Name: "ssh", + Inside: 22, + }, + 9339: { + Names: []string{"gnmi", "gnoi"}, + Inside: 57400, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + { + desc: "unsupported service only", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 179: { + Name: "bgp", + Inside: 179, + }, + }, + }, + want: nil, + }, + { + desc: "service with zero inside falls back to map key", + node: &topopb.Node{ + Services: map[uint32]*topopb.Service{ + 22: { + Name: "ssh", + Inside: 0, + }, + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + got := ServiceReadinessProbe(tt.node) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("ServiceReadinessProbe() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/topo/node/nokia/nokia.go b/topo/node/nokia/nokia.go index 72cef344..a9df2713 100644 --- a/topo/node/nokia/nokia.go +++ b/topo/node/nokia/nokia.go @@ -141,7 +141,7 @@ func (n *Node) GenerateSelfSigned(ctx context.Context) error { return nil } log.Infof("%s - generating self signed certs", n.Name()) - log.Infof("%s - waiting for pod to be running", n.Name()) + log.Infof("%s - waiting for pod to be ready", n.Name()) w, err := n.KubeClient.CoreV1().Pods(n.Namespace).Watch(ctx, metav1.ListOptions{ FieldSelector: fields.SelectorFromSet( fields.Set{metav1.ObjectNameField: n.Name()}, @@ -156,10 +156,19 @@ func (n *Node) GenerateSelfSigned(ctx context.Context) error { continue } if p.Status.Phase == corev1.PodRunning { - break + var ready bool + for _, cond := range p.Status.Conditions { + if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue { + ready = true + break + } + } + if ready { + break + } } } - log.Infof("%s - pod running.", n.Name()) + log.Infof("%s - pod ready.", n.Name()) if err := n.SpawnCLIConn(); err != nil { return err diff --git a/topo/node/nokia/nokia_test.go b/topo/node/nokia/nokia_test.go index dfeea873..c25a1e8a 100644 --- a/topo/node/nokia/nokia_test.go +++ b/topo/node/nokia/nokia_test.go @@ -210,6 +210,10 @@ func TestGenerateSelfSigned(t *testing.T) { Object: &corev1.Pod{ Status: corev1.PodStatus{ Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }}, }, }, }}, diff --git a/topo/node/sonic/sonic.go b/topo/node/sonic/sonic.go index f62f4e11..df2be70f 100644 --- a/topo/node/sonic/sonic.go +++ b/topo/node/sonic/sonic.go @@ -122,6 +122,7 @@ func (n *Node) CreatePod(ctx context.Context) error { SecurityContext: &corev1.SecurityContext{ Privileged: pointer.Bool(true), }, + ReadinessProbe: node.ServiceReadinessProbe(pb), }} pod := &corev1.Pod{ diff --git a/topo/node/sonic/sonic_test.go b/topo/node/sonic/sonic_test.go index db6dd1e9..9ea43856 100644 --- a/topo/node/sonic/sonic_test.go +++ b/topo/node/sonic/sonic_test.go @@ -26,6 +26,7 @@ import ( "google.golang.org/protobuf/proto" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" kfake "k8s.io/client-go/kubernetes/fake" "k8s.io/utils/pointer" ) @@ -280,6 +281,56 @@ func TestCreatePod(t *testing.T) { SubPath: "config_db.json", }}, }, + }, { + desc: "sonic container with ssh service has readiness probe", + nImpl: &node.Impl{ + Proto: &tpb.Node{ + Name: "sonic-node", + Config: &tpb.Config{ + Image: "sonicImage", + Command: []string{"sonicCommand"}, + Args: []string{"sonicArgs"}, + }, + Services: map[uint32]*tpb.Service{ + 22: { + Name: "ssh", + Inside: 22, + }, + }, + }, + }, + wantInitCtr: corev1.Container{ + Name: "init-sonic-node", + Image: node.DefaultInitContainerImage, + Args: []string{"1", "0", "1"}, + ImagePullPolicy: "IfNotPresent", + SecurityContext: &corev1.SecurityContext{ + Privileged: pointer.Bool(true), + }, + }, + wantSonicCtr: corev1.Container{ + Name: "sonic-node", + Image: "sonicImage", + Command: []string{"sonicCommand"}, + Args: []string{"sonicArgs"}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{}, + }, + ImagePullPolicy: "IfNotPresent", + SecurityContext: &corev1.SecurityContext{ + Privileged: pointer.Bool(true), + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(22), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + FailureThreshold: 60, + }, + }, }} for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) {