Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

Commit dd42d04

Browse files
shashankramshashankram
authored
cli/verifier: add control plane health probe checks (#4751)
- Adds HTTP and HTTPS liveness probe checks to control plane health verification command - Moves unnecessary constants pkg in the generic httpserver directory and consolidates those into the constants pkg - Fixes a bug in the existing proxy_get port-forwarding code used by the verifier, where a connection leak exists due to not closing the response - Hides the probe implementation behind an interface so that the verifier can be unit tested without needing to hack the port-forwarding dependency - Consolidates /healthz constants for webhooks Part of #4634 Signed-off-by: Shashank Ram <[email protected]>
1 parent 0af42df commit dd42d04

File tree

21 files changed

+383
-90
lines changed

21 files changed

+383
-90
lines changed

‎cmd/cli/util.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"k8s.io/client-go/rest"
2121

2222
"github.com/openservicemesh/osm/pkg/constants"
23-
httpserverconstants "github.com/openservicemesh/osm/pkg/httpserver/constants"
2423
"github.com/openservicemesh/osm/pkg/k8s"
2524
)
2625

@@ -214,7 +213,7 @@ func getSupportedSmiForControllerPod(pod string, namespace string, restConfig *r
214213

215214
err = portForwarder.Start(func(pf *k8s.PortForwarder) error {
216215
defer pf.Stop()
217-
url := fmt.Sprintf("http://localhost:%d%s", localPort, httpserverconstants.SmiVersionPath)
216+
url := fmt.Sprintf("http://localhost:%d%s", localPort, constants.OSMControllerSMIVersionPath)
218217

219218
// #nosec G107: Potential HTTP request made with variable url
220219
resp, err := http.Get(url)

‎cmd/cli/verify_control_plane.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func newVerifyControlPlaneCmd(stdout io.Writer, stderr io.Writer) *cobra.Command
6060
}
6161

6262
func (cmd *verifyControlPlaneCmd) run() error {
63-
v := verifier.NewControlPlaneHealthVerifier(cmd.stdout, cmd.stderr, cmd.kubeClient, settings.Namespace())
63+
v := verifier.NewControlPlaneHealthVerifier(cmd.stdout, cmd.stderr, cmd.kubeClient, cmd.restConfig, settings.Namespace())
6464
result := v.Run()
6565

6666
fmt.Fprintln(cmd.stdout, "---------------------------------------------")

‎cmd/cli/version.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"k8s.io/client-go/rest"
1515

1616
"github.com/openservicemesh/osm/pkg/constants"
17-
httpserverconstants "github.com/openservicemesh/osm/pkg/httpserver/constants"
1817
"github.com/openservicemesh/osm/pkg/version"
1918
)
2019

@@ -153,7 +152,7 @@ func (v *versionCmd) getMeshVersion() (*remoteVersionInfo, error) {
153152
}
154153

155154
func (r *remoteVersion) proxyGetMeshVersion(pod string, namespace string, clientset kubernetes.Interface) (*version.Info, error) {
156-
resp, err := clientset.CoreV1().Pods(namespace).ProxyGet("", pod, strconv.Itoa(constants.OSMHTTPServerPort), httpserverconstants.VersionPath, nil).DoRaw(context.TODO())
155+
resp, err := clientset.CoreV1().Pods(namespace).ProxyGet("", pod, strconv.Itoa(constants.OSMHTTPServerPort), constants.VersionPath, nil).DoRaw(context.TODO())
157156
if err != nil {
158157
return nil, errors.Wrapf(err, "Error retrieving mesh version from pod [%s] in namespace [%s]", pod, namespace)
159158
}

‎cmd/osm-bootstrap/osm-bootstrap.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,13 @@ import (
2727
"k8s.io/kubectl/pkg/util"
2828

2929
configv1alpha2 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha2"
30+
configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
3031

3132
"github.com/openservicemesh/osm/pkg/certificate/providers"
3233
"github.com/openservicemesh/osm/pkg/configurator"
3334
"github.com/openservicemesh/osm/pkg/constants"
3435
"github.com/openservicemesh/osm/pkg/crdconversion"
35-
configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
3636
"github.com/openservicemesh/osm/pkg/httpserver"
37-
httpserverconstants "github.com/openservicemesh/osm/pkg/httpserver/constants"
3837
"github.com/openservicemesh/osm/pkg/k8s/events"
3938
"github.com/openservicemesh/osm/pkg/logger"
4039
"github.com/openservicemesh/osm/pkg/messaging"
@@ -219,9 +218,9 @@ func main() {
219218
*/
220219
httpServer := httpserver.NewHTTPServer(constants.OSMHTTPServerPort)
221220
// Metrics
222-
httpServer.AddHandler(httpserverconstants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
221+
httpServer.AddHandler(constants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
223222
// Version
224-
httpServer.AddHandler(httpserverconstants.VersionPath, version.GetVersionHandler())
223+
httpServer.AddHandler(constants.VersionPath, version.GetVersionHandler())
225224
// Start HTTP server
226225
err = httpServer.Start()
227226
if err != nil {

‎cmd/osm-controller/osm-controller.go

+8-9
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import (
2727

2828
configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
2929
policyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned"
30-
"github.com/openservicemesh/osm/pkg/messaging"
31-
"github.com/openservicemesh/osm/pkg/reconciler"
3230

3331
"github.com/openservicemesh/osm/pkg/catalog"
3432
"github.com/openservicemesh/osm/pkg/certificate/providers"
@@ -42,14 +40,15 @@ import (
4240
"github.com/openservicemesh/osm/pkg/errcode"
4341
"github.com/openservicemesh/osm/pkg/health"
4442
"github.com/openservicemesh/osm/pkg/httpserver"
45-
httpserverconstants "github.com/openservicemesh/osm/pkg/httpserver/constants"
4643
"github.com/openservicemesh/osm/pkg/ingress"
4744
"github.com/openservicemesh/osm/pkg/k8s"
4845
"github.com/openservicemesh/osm/pkg/k8s/events"
4946
"github.com/openservicemesh/osm/pkg/logger"
47+
"github.com/openservicemesh/osm/pkg/messaging"
5048
"github.com/openservicemesh/osm/pkg/metricsstore"
5149
"github.com/openservicemesh/osm/pkg/policy"
5250
"github.com/openservicemesh/osm/pkg/providers/kube"
51+
"github.com/openservicemesh/osm/pkg/reconciler"
5352
"github.com/openservicemesh/osm/pkg/service"
5453
"github.com/openservicemesh/osm/pkg/signals"
5554
"github.com/openservicemesh/osm/pkg/smi"
@@ -278,15 +277,15 @@ func main() {
278277
// Health/Liveness probes
279278
funcProbes := []health.Probes{xdsServer, smi.HealthChecker{DiscoveryClient: clientset.Discovery()}}
280279
httpServer.AddHandlers(map[string]http.Handler{
281-
httpserverconstants.HealthReadinessPath: health.ReadinessHandler(funcProbes, getHTTPHealthProbes()),
282-
httpserverconstants.HealthLivenessPath: health.LivenessHandler(funcProbes, getHTTPHealthProbes()),
280+
constants.OSMControllerReadinessPath: health.ReadinessHandler(funcProbes, getHTTPHealthProbes()),
281+
constants.OSMControllerLivenessPath: health.LivenessHandler(funcProbes, getHTTPHealthProbes()),
283282
})
284283
// Metrics
285-
httpServer.AddHandler(httpserverconstants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
284+
httpServer.AddHandler(constants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
286285
// Version
287-
httpServer.AddHandler(httpserverconstants.VersionPath, version.GetVersionHandler())
286+
httpServer.AddHandler(constants.VersionPath, version.GetVersionHandler())
288287
// Supported SMI Versions
289-
httpServer.AddHandler(httpserverconstants.SmiVersionPath, smi.GetSmiClientVersionHTTPHandler())
288+
httpServer.AddHandler(constants.OSMControllerSMIVersionPath, smi.GetSmiClientVersionHTTPHandler())
290289

291290
// Start HTTP server
292291
err = httpServer.Start()
@@ -344,7 +343,7 @@ func getHTTPHealthProbes() []health.HTTPProbe {
344343
return []health.HTTPProbe{
345344
// Internal probe to validator's webhook port
346345
{
347-
URL: joinURL(fmt.Sprintf("https://%s:%d", constants.LocalhostIPAddress, constants.ValidatorWebhookPort), validator.HealthAPIPath),
346+
URL: joinURL(fmt.Sprintf("https://%s:%d", constants.LocalhostIPAddress, constants.ValidatorWebhookPort), constants.WebhookHealthPath),
348347
Protocol: health.ProtocolHTTPS,
349348
},
350349
}

‎cmd/osm-injector/osm-injector.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,19 @@ import (
2323

2424
configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
2525
policyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned"
26-
"github.com/openservicemesh/osm/pkg/messaging"
27-
"github.com/openservicemesh/osm/pkg/reconciler"
2826

2927
"github.com/openservicemesh/osm/pkg/certificate/providers"
3028
"github.com/openservicemesh/osm/pkg/configurator"
3129
"github.com/openservicemesh/osm/pkg/constants"
3230
"github.com/openservicemesh/osm/pkg/errcode"
3331
"github.com/openservicemesh/osm/pkg/httpserver"
34-
httpserverconstants "github.com/openservicemesh/osm/pkg/httpserver/constants"
3532
"github.com/openservicemesh/osm/pkg/injector"
3633
"github.com/openservicemesh/osm/pkg/k8s"
3734
"github.com/openservicemesh/osm/pkg/k8s/events"
3835
"github.com/openservicemesh/osm/pkg/logger"
36+
"github.com/openservicemesh/osm/pkg/messaging"
3937
"github.com/openservicemesh/osm/pkg/metricsstore"
38+
"github.com/openservicemesh/osm/pkg/reconciler"
4039
"github.com/openservicemesh/osm/pkg/signals"
4140
"github.com/openservicemesh/osm/pkg/version"
4241
)
@@ -209,9 +208,9 @@ func main() {
209208
*/
210209
httpServer := httpserver.NewHTTPServer(constants.OSMHTTPServerPort)
211210
// Metrics
212-
httpServer.AddHandler(httpserverconstants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
211+
httpServer.AddHandler(constants.MetricsPath, metricsstore.DefaultMetricsStore.Handler())
213212
// Version
214-
httpServer.AddHandler(httpserverconstants.VersionPath, version.GetVersionHandler())
213+
httpServer.AddHandler(constants.VersionPath, version.GetVersionHandler())
215214
// Start HTTP server
216215
err = httpServer.Start()
217216
if err != nil {

‎pkg/cli/proxy_get.go

+3
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ func GetEnvoyProxyConfig(clientSet kubernetes.Interface, config *rest.Config, na
5151
if err != nil {
5252
return errors.Errorf("Error fetching url %s: %s", url, err)
5353
}
54+
//nolint: errcheck
55+
//#nosec G307
56+
defer resp.Body.Close()
5457

5558
envoyProxyConfig, err = ioutil.ReadAll(resp.Body)
5659
if err != nil {

‎pkg/cli/verifier/control_plane.go

+42-6
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,53 @@ import (
55

66
"k8s.io/apimachinery/pkg/types"
77
"k8s.io/client-go/kubernetes"
8+
"k8s.io/client-go/rest"
89

910
"github.com/openservicemesh/osm/pkg/constants"
11+
"github.com/openservicemesh/osm/pkg/crdconversion"
1012
)
1113

1214
// ControlPlaneHealthVerifier implements the Verifier interface for control plane health
1315
type ControlPlaneHealthVerifier struct {
14-
stdout io.Writer
15-
stderr io.Writer
16-
kubeClient kubernetes.Interface
17-
namespace string
16+
stdout io.Writer
17+
stderr io.Writer
18+
kubeClient kubernetes.Interface
19+
restConfig *rest.Config
20+
namespace string
21+
controllerProber httpProber
22+
injectorProber httpProber
23+
bootstrapProber httpProber
1824
}
1925

2026
// NewControlPlaneHealthVerifier implements verification for control plane health
21-
func NewControlPlaneHealthVerifier(stdout io.Writer, stderr io.Writer, kubeClient kubernetes.Interface, namespace string) Verifier {
27+
func NewControlPlaneHealthVerifier(stdout io.Writer, stderr io.Writer, kubeClient kubernetes.Interface, restConfig *rest.Config, namespace string) Verifier {
2228
return &ControlPlaneHealthVerifier{
2329
stdout: stdout,
2430
stderr: stderr,
2531
kubeClient: kubeClient,
32+
restConfig: restConfig,
2633
namespace: namespace,
34+
controllerProber: &podProber{
35+
kubeClient: kubeClient,
36+
restConfig: restConfig,
37+
port: constants.OSMHTTPServerPort,
38+
path: constants.OSMControllerLivenessPath,
39+
protocol: constants.ProtocolHTTP,
40+
},
41+
injectorProber: &podProber{
42+
kubeClient: kubeClient,
43+
restConfig: restConfig,
44+
port: constants.InjectorWebhookPort,
45+
path: constants.WebhookHealthPath,
46+
protocol: constants.ProtocolHTTPS,
47+
},
48+
bootstrapProber: &podProber{
49+
kubeClient: kubeClient,
50+
restConfig: restConfig,
51+
port: crdconversion.HealthzPort,
52+
path: constants.WebhookHealthPath,
53+
protocol: constants.ProtocolHTTP,
54+
},
2755
}
2856
}
2957

@@ -32,10 +60,18 @@ func (v *ControlPlaneHealthVerifier) Run() Result {
3260
ctx := "Verify the health of OSM control plane"
3361

3462
verifiers := Set{
35-
// Pod status verification
63+
// Pod readiness verification
3664
NewPodStatusVerifier(v.stdout, v.stderr, v.kubeClient, types.NamespacedName{Namespace: v.namespace, Name: constants.OSMControllerName}),
3765
NewPodStatusVerifier(v.stdout, v.stderr, v.kubeClient, types.NamespacedName{Namespace: v.namespace, Name: constants.OSMInjectorName}),
3866
NewPodStatusVerifier(v.stdout, v.stderr, v.kubeClient, types.NamespacedName{Namespace: v.namespace, Name: constants.OSMBootstrapName}),
67+
68+
// Pod liveness health probe verification
69+
NewPodProbeVerifier(v.stdout, v.stderr, v.kubeClient,
70+
types.NamespacedName{Namespace: v.namespace, Name: constants.OSMControllerName}, v.controllerProber),
71+
NewPodProbeVerifier(v.stdout, v.stderr, v.kubeClient,
72+
types.NamespacedName{Namespace: v.namespace, Name: constants.OSMInjectorName}, v.injectorProber),
73+
NewPodProbeVerifier(v.stdout, v.stderr, v.kubeClient,
74+
types.NamespacedName{Namespace: v.namespace, Name: constants.OSMBootstrapName}, v.bootstrapProber),
3975
}
4076

4177
return verifiers.Run(ctx)

‎pkg/cli/verifier/control_plane_test.go

+105-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package verifier
22

33
import (
44
"bytes"
5+
"errors"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -17,9 +18,12 @@ func TestControlPlane(t *testing.T) {
1718
testNs := "test"
1819

1920
testCases := []struct {
20-
name string
21-
resources []runtime.Object
22-
expected Result
21+
name string
22+
resources []runtime.Object
23+
controllerProbeErr error
24+
injectorProbeErr error
25+
bootstrapProbeErr error
26+
expected Result
2327
}{
2428
{
2529
name: "all control plane pods are ready",
@@ -214,16 +218,112 @@ func TestControlPlane(t *testing.T) {
214218
Status: Failure,
215219
},
216220
},
221+
{
222+
name: "control plane probes fails",
223+
resources: []runtime.Object{
224+
&corev1.Pod{
225+
ObjectMeta: metav1.ObjectMeta{
226+
Name: "osm-controller-1",
227+
Namespace: testNs,
228+
Labels: map[string]string{constants.AppLabel: "osm-controller"},
229+
},
230+
Status: corev1.PodStatus{
231+
Conditions: []corev1.PodCondition{
232+
{
233+
Type: corev1.PodScheduled,
234+
Status: corev1.ConditionTrue,
235+
},
236+
{
237+
Type: corev1.ContainersReady,
238+
Status: corev1.ConditionTrue,
239+
},
240+
{
241+
Type: corev1.PodInitialized,
242+
Status: corev1.ConditionTrue,
243+
},
244+
{
245+
Type: corev1.PodReady,
246+
Status: corev1.ConditionTrue,
247+
},
248+
},
249+
},
250+
},
251+
&corev1.Pod{
252+
ObjectMeta: metav1.ObjectMeta{
253+
Name: "osm-injector-1",
254+
Namespace: testNs,
255+
Labels: map[string]string{constants.AppLabel: "osm-injector"},
256+
},
257+
Status: corev1.PodStatus{
258+
Conditions: []corev1.PodCondition{
259+
{
260+
Type: corev1.PodScheduled,
261+
Status: corev1.ConditionTrue,
262+
},
263+
{
264+
Type: corev1.ContainersReady,
265+
Status: corev1.ConditionTrue,
266+
},
267+
{
268+
Type: corev1.PodInitialized,
269+
Status: corev1.ConditionTrue,
270+
},
271+
{
272+
Type: corev1.PodReady,
273+
Status: corev1.ConditionTrue,
274+
},
275+
},
276+
},
277+
},
278+
&corev1.Pod{
279+
ObjectMeta: metav1.ObjectMeta{
280+
Name: "osm-bootstrap-1",
281+
Namespace: testNs,
282+
Labels: map[string]string{constants.AppLabel: "osm-bootstrap"},
283+
},
284+
Status: corev1.PodStatus{
285+
Conditions: []corev1.PodCondition{
286+
{
287+
Type: corev1.PodScheduled,
288+
Status: corev1.ConditionTrue,
289+
},
290+
{
291+
Type: corev1.ContainersReady,
292+
Status: corev1.ConditionTrue,
293+
},
294+
{
295+
Type: corev1.PodInitialized,
296+
Status: corev1.ConditionTrue,
297+
},
298+
{
299+
Type: corev1.PodReady,
300+
Status: corev1.ConditionTrue,
301+
},
302+
},
303+
},
304+
},
305+
},
306+
controllerProbeErr: errors.New("fake error"), // fake probe error
307+
injectorProbeErr: errors.New("fake error"), // fake probe error
308+
bootstrapProbeErr: errors.New("fake error"), // fake probe error
309+
expected: Result{
310+
Status: Failure,
311+
},
312+
},
217313
}
218314

219315
for _, tc := range testCases {
220316
t.Run(tc.name, func(t *testing.T) {
221317
a := assert.New(t)
222318

223319
fakeClient := fake.NewSimpleClientset(tc.resources...)
320+
224321
v := &ControlPlaneHealthVerifier{
225-
kubeClient: fakeClient,
226-
namespace: testNs,
322+
kubeClient: fakeClient,
323+
namespace: testNs,
324+
controllerProber: fakeHTTPProber{err: tc.controllerProbeErr},
325+
injectorProber: fakeHTTPProber{err: tc.injectorProbeErr},
326+
bootstrapProber: fakeHTTPProber{err: tc.bootstrapProbeErr},
227327
}
228328

229329
actual := v.Run()

0 commit comments

Comments
 (0)