Liveness probe #309
Liveness probe #309
Conversation
…ia readiness probe
| @@ -103,7 +103,7 @@ spec: | |||
| name: iptableslock | |||
| containers: | |||
| - name: nmi | |||
| image: "mcr.microsoft.com/k8s/aad-pod-identity/nmi:1.5-rc1" | |||
| image: "testle5.azurecr.io/nmi:le" | |||
| @@ -191,7 +203,7 @@ spec: | |||
| serviceAccountName: aad-pod-id-mic-service-account | |||
| containers: | |||
| - name: mic | |||
| image: "mcr.microsoft.com/k8s/aad-pod-identity/mic:1.5-rc1" | |||
| image: "testle5.azurecr.io/mic:le" | |||
| var err error | ||
| var t *template.Template | ||
| if !old { | ||
| t, err = template.New("deployment-rbac.yaml").ParseFiles(path.Join("template", "deployment-rbac.yaml")) |
There was a problem hiding this comment.
Rather than having 2 different deployment files, we should just template the single file to include new specs/ignore based on a boolean flag.
There was a problem hiding this comment.
That sounds good. Will take it up in a future PR.
| AfterEach(func() { | ||
| fmt.Println("\nTearing down the test environment...") | ||
|
|
||
| if CurrentGinkgoTestDescription().Failed { |
There was a problem hiding this comment.
I don't think we should skip cleanup if there is a failure. Instead we should dump the logs here. If we skip clean up, it can cause issues in automation.
There was a problem hiding this comment.
Yes I agree. This is stop gap before going to the full copy logs and events atleast until that PR is done.
| @@ -573,7 +590,7 @@ var _ = Describe("Kubernetes cluster using aad-pod-identity", func() { | |||
| Expect(ok).To(Equal(true)) | |||
|
|
|||
| // setup mic and nmi with old releases | |||
| setupInfra("mcr.microsoft.com/k8s/aad-pod-identity", "1.4", "1.3") | |||
| setupInfraOld("mcr.microsoft.com/k8s/aad-pod-identity", "1.4", "1.3") | |||
There was a problem hiding this comment.
I think instead of 2 functions we should just pass a boolean and use a single template to generate the final deployment.
There was a problem hiding this comment.
Did not want to make changes in setupinfra all through, instead kept the changes to internal function only.
There was a problem hiding this comment.
Ideally we should generate the required template based on version, <= 1.4. When that change is made in future PR, this single instance and setupold function will go away.
| port: 8080 | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 5 | ||
| readinessProbe: |
There was a problem hiding this comment.
ReadinessProbe is typically used to see if an endpoint is ready to receive traffic. Only when the readinessprobe passes the pod ip is added to list of endpoints in the service that is exposing the pod. We don't have a service with mic or nmi. Is there a reason we are doing readiness probe too?
cmd/mic/main.go
Outdated
| @@ -39,6 +41,9 @@ func main() { | |||
| flag.StringVar(&leaderElectionCfg.Name, "leader-election-name", "aad-pod-identity-mic", "leader election name") | |||
| flag.DurationVar(&leaderElectionCfg.Duration, "leader-election-duration", time.Second*15, "leader election duration") | |||
|
|
|||
| //Probe port | |||
| flag.StringVar(&httpProbePort, "http-probe-port", "8080", "http health and ready probe port") | |||
There was a problem hiding this comment.
nit: http health and liveness probe port
pkg/nmi/server/server.go
Outdated
| @@ -91,6 +103,11 @@ func (s *Server) updateIPTableRules() { | |||
| ticker := time.NewTicker(time.Second * time.Duration(s.IPTableUpdateTimeIntervalInSeconds)) | |||
| defer ticker.Stop() | |||
|
|
|||
| // Run once before the waiting on ticket for the rules to take effect | |||
|
Ran 13 of 13 Specs in 2388.736 seconds |
Reason for Change:
- Add readiness at /ready endpoint for MIC and NMI at 8080 by default.- Both liveness and readiness probe return back 200 okay as soon as they are activated from the current run.Issue Fixed:
#288
Notes for Reviewers: