From 21bc98979e2c21df4da34142248ca741b3ccf3da Mon Sep 17 00:00:00 2001 From: Inteon <42113979+inteon@users.noreply.github.com> Date: Fri, 16 Jul 2021 13:11:40 +0200 Subject: [PATCH] improved ux Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com> --- cmd/ctl/pkg/check/api/api.go | 40 ++++--- devel/addon/certmanager/install.sh | 5 +- pkg/util/cmapichecker/cmapichecker.go | 133 ++++++++++----------- pkg/util/cmapichecker/cmapichecker_test.go | 127 +++++++++++++------- 4 files changed, 177 insertions(+), 128 deletions(-) diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go index 67b503322..533138e23 100644 --- a/cmd/ctl/pkg/check/api/api.go +++ b/cmd/ctl/pkg/check/api/api.go @@ -18,7 +18,10 @@ package api import ( "context" + "errors" "fmt" + "log" + "os" "time" "github.com/spf13/cobra" @@ -98,50 +101,57 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams, cmd := &cobra.Command{ Use: "api", - Short: "This check attempts to perform a dry-run create of a cert-manager Certificate", + Short: "Check if the cert-manager API is ready", Long: checkApiDesc, RunE: func(cmd *cobra.Command, args []string) error { if err := o.Complete(factory); err != nil { return err } - return o.Run(ctx) + o.Run(ctx) + return nil }, SilenceUsage: true, SilenceErrors: true, } - cmd.Flags().DurationVar(&o.Wait, "wait", 1*time.Minute, "Time before timeout when waiting, must include unit, e.g. 0s or 20s") + cmd.Flags().DurationVar(&o.Wait, "wait", 0, "Wait until the cert-manager API is ready (default 0s)") cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 1m or 10m") - cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print details regarding encountered errors") + cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print detailed error messages") return cmd } // Run executes check api command -func (o *Options) Run(ctx context.Context) error { +func (o *Options) Run(ctx context.Context) { + if !o.Verbose { + log.SetFlags(0) // Disable prefixing logs with timestamps. + } + log.SetOutput(o.ErrOut) // Log all intermediate errors to stderr + pollContext, cancel := context.WithTimeout(ctx, o.Wait) defer cancel() pollErr := wait.PollImmediateUntil(o.Interval, func() (done bool, err error) { if err := o.APIChecker.Check(ctx); err != nil { - if o.Verbose { - fmt.Fprintf(o.ErrOut, "Not ready: %v (%v)\n", err, err.Cause()) - } else { - fmt.Fprintf(o.ErrOut, "Not ready: %v\n", err) + if !o.Verbose && errors.Unwrap(err) != nil { + err = errors.Unwrap(err) } + + log.Printf("Not ready: %v", err) return false, nil } - fmt.Fprintln(o.Out, "The cert-manager API is ready") - return true, nil }, pollContext.Done()) + log.SetOutput(o.Out) // Log conclusion to stdout + if pollErr != nil { - if ctx.Err() != nil { - return ctx.Err() + if errors.Is(pollContext.Err(), context.DeadlineExceeded) && o.Wait > 0 { + log.Printf("Timed out after %s", o.Wait) } - return pollErr + + os.Exit(1) } - return nil + log.Printf("The cert-manager API is ready") } diff --git a/devel/addon/certmanager/install.sh b/devel/addon/certmanager/install.sh index 1a151a30f..aaa47f56b 100755 --- a/devel/addon/certmanager/install.sh +++ b/devel/addon/certmanager/install.sh @@ -53,6 +53,9 @@ kubectl get namespace "${NAMESPACE}" || kubectl create namespace "${NAMESPACE}" # Build the Helm chart package .tgz bazel build //deploy/charts/cert-manager +# Pre-compile the kubectl plugin, so it can quickly check the api status +bazel build //hack/bin:kubectl-cert_manager + # Upgrade or install cert-manager helm upgrade \ --install \ @@ -67,4 +70,4 @@ helm upgrade \ "$RELEASE_NAME" \ "$REPO_ROOT/bazel-bin/deploy/charts/cert-manager/cert-manager.tgz" -kubectl cert-manager check api +kubectl cert-manager check api --wait=1m -v diff --git a/pkg/util/cmapichecker/cmapichecker.go b/pkg/util/cmapichecker/cmapichecker.go index 4a3fde241..e9dbd6477 100644 --- a/pkg/util/cmapichecker/cmapichecker.go +++ b/pkg/util/cmapichecker/cmapichecker.go @@ -18,6 +18,7 @@ package cmapichecker import ( "context" + "fmt" "regexp" errors "github.com/pkg/errors" @@ -35,43 +36,30 @@ import ( ) var ( - ErrAPIServerUnreachable = errors.New("unable to connect to the Kubernetes API server") - ErrCertManagerCRDsNotFound = errors.New("the cert-manager CRDs are not yet installed on the Kubernetes API server") - ErrCertManagerAPIEndpointsNotEstablished = errors.New("the cert-manager API endpoints have not yet been published by the Kubernetes API server") - ErrWebhookConnectionFailure = errors.New("the cert-manager webhook server can't be reached yet") - ErrWebhookCertificateFailure = errors.New("the client CA bundle is not yet updated to the certificate of the cert-manager webhook") - - regexErrCertManagerCRDsNotFound = regexp.MustCompile(`^error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"$`) - regexErrCertManagerAPIEndpointsNotEstablished = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": service "(.*)-webhook" not found$`) - regexErrWebhookConnectionFailure = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": (.*): connect: connection refused$`) - regexErrWebhookCertificateFailure = regexp.MustCompile(`Post "(.*)": x509: certificate signed by unknown authority`) + ErrCertManagerCRDsNotFound = errors.New("the cert-manager CRDs are not yet installed on the Kubernetes API server") + ErrWebhookServiceFailure = errors.New("the cert-manager webhook service is not created yet") + ErrWebhookDeploymentFailure = errors.New("the cert-manager webhook deployment is not ready yet") + ErrWebhookCertificateFailure = errors.New("the cert-manager webhook CA bundle is not injected yet") ) -type ApiCheckError struct { - SimpleError error - UnderlyingError error -} +const ( + crdsMappingError = `error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"` + crdsNotFoundError = `the server could not find the requested resource (post certificates.cert-manager.io)` +) -func (e *ApiCheckError) Error() string { - return e.SimpleError.Error() -} - -func (e *ApiCheckError) Cause() error { - return e.UnderlyingError -} +var ( + regexErrCertManagerCRDsNotFound = regexp.MustCompile(`^(` + regexp.QuoteMeta(crdsMappingError) + `|` + regexp.QuoteMeta(crdsNotFoundError) + `)$`) + regexErrWebhookServiceFailure = regexp.MustCompile(`Post "(.*)": service "(.*)-webhook" not found`) + regexErrWebhookDeploymentFailure = regexp.MustCompile(`Post "(.*)": (.*): connect: connection refused`) + regexErrWebhookCertificateFailure = regexp.MustCompile(`Post "(.*)": x509: certificate signed by unknown authority`) +) // Interface is used to check that the cert-manager CRDs have been installed and are usable. type Interface interface { - Check(context.Context) *ApiCheckError + Check(context.Context) error } type cmapiChecker struct { - // The client controller-runtime client.New function fails if can't reach - // the API server, so we load it lazily, to avoid breaking integration tests - // which rely on being able to start the webhook server before the API - // server. - clientBuilder func() (client.Client, error) - client client.Client } @@ -80,31 +68,17 @@ func New(restcfg *rest.Config, scheme *runtime.Scheme, namespace string) (Interf if err := cmapi.AddToScheme(scheme); err != nil { return nil, errors.Wrap(err, "while configuring scheme") } - return &cmapiChecker{ - clientBuilder: func() (client.Client, error) { - cl, err := client.New(restcfg, client.Options{ - Scheme: scheme, - }) - if err != nil { - return nil, errors.Wrap(err, "while creating client") - } - return client.NewNamespacedClient(client.NewDryRunClient(cl), namespace), nil - }, - }, nil -} -func (o *cmapiChecker) Client() (client.Client, error) { - if o.client != nil { - return o.client, nil - } - - cl, err := o.clientBuilder() + cl, err := client.New(restcfg, client.Options{ + Scheme: scheme, + }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "while creating client") } - o.client = cl - return o.client, nil + return &cmapiChecker{ + client: client.NewNamespacedClient(client.NewDryRunClient(cl), namespace), + }, nil } // Check attempts to perform a dry-run create of a cert-manager *v1alpha2* @@ -112,7 +86,7 @@ func (o *cmapiChecker) Client() (client.Client, error) { // required webhooks are reachable by the K8S API server. // We use v1alpha2 API to ensure that the API server has also connected to the // cert-manager conversion webhook. -func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { +func (o *cmapiChecker) Check(ctx context.Context) error { cert := &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "cmapichecker-", @@ -126,21 +100,7 @@ func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { }, } - // while creating client: Get "http://localhost:8080/api?timeout=32s": dial tcp 127.0.0.1:8080: connect: connection refused - cl, err := o.Client() - if err != nil { - return &ApiCheckError{ - SimpleError: ErrAPIServerUnreachable, - UnderlyingError: err, - } - } - - // error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io" - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca") - // conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority - if err := cl.Create(ctx, cert); err != nil { + if err := o.client.Create(ctx, cert); err != nil { return &ApiCheckError{ SimpleError: translateToSimpleError(err), UnderlyingError: err, @@ -149,18 +109,51 @@ func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { return nil } +type ApiCheckError struct { + SimpleError error + UnderlyingError error +} + +func (e *ApiCheckError) Error() string { + // If no simple error exists, print underlying error + if e.SimpleError == nil { + return e.UnderlyingError.Error() + } + return fmt.Sprintf("%v (%v)", e.SimpleError.Error(), e.UnderlyingError.Error()) +} + +// If no simple error exists, this function will return nil +// which indicates that the error is not unwrappable +func (e *ApiCheckError) Unwrap() error { + return e.SimpleError +} + +// This translateToSimpleError function detects errors based on the error message. +// It tries to map these error messages to a better understandable error message that +// explains what is wrong. If it cannot create a simple error, it will return nil. +// ErrCertManagerCRDsNotFound: +// - error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io" +// ErrWebhookServiceFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": service "cert-manager-webhook" not found +// ErrWebhookDeploymentFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": dial tcp 10.96.38.90:443: connect: connection refused +// ErrWebhookCertificateFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca") +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority func translateToSimpleError(err error) error { s := err.Error() if regexErrCertManagerCRDsNotFound.MatchString(s) { return ErrCertManagerCRDsNotFound - } else if regexErrCertManagerAPIEndpointsNotEstablished.MatchString(s) { - return ErrCertManagerAPIEndpointsNotEstablished - } else if regexErrWebhookConnectionFailure.MatchString(s) { - return ErrWebhookConnectionFailure + } else if regexErrWebhookServiceFailure.MatchString(s) { + return ErrWebhookServiceFailure + } else if regexErrWebhookDeploymentFailure.MatchString(s) { + return ErrWebhookDeploymentFailure } else if regexErrWebhookCertificateFailure.MatchString(s) { return ErrWebhookCertificateFailure } - return err + return nil } diff --git a/pkg/util/cmapichecker/cmapichecker_test.go b/pkg/util/cmapichecker/cmapichecker_test.go index 0d381fcdb..cd8f5638e 100644 --- a/pkg/util/cmapichecker/cmapichecker_test.go +++ b/pkg/util/cmapichecker/cmapichecker_test.go @@ -19,6 +19,7 @@ package cmapichecker import ( "context" "errors" + "fmt" "testing" "k8s.io/apimachinery/pkg/runtime" @@ -31,7 +32,6 @@ import ( type fakeErrorClient struct { client.Client - newError error createError error } @@ -51,63 +51,91 @@ func newFakeCmapiChecker() (*fakeErrorClient, Interface, error) { cl := fake.NewClientBuilder().WithScheme(scheme).Build() errorClient := &fakeErrorClient{ Client: cl, - newError: nil, createError: nil, } return errorClient, &cmapiChecker{ - clientBuilder: func() (client.Client, error) { - if errorClient.newError != nil { - return nil, errorClient.newError - } - return errorClient, nil - }, + client: errorClient, }, nil } +const ( + errCertManagerCRDsMapping = `error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"` + errCertManagerCRDsNotFound = `the server could not find the requested resource (post certificates.cert-manager.io)` + + errMutatingWebhookServiceFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found` + errMutatingWebhookDeploymentFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused` + errMutatingWebhookCertificateFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca"` + + errConversionWebhookServiceFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": service "cert-manager-webhook" not found` + errConversionWebhookDeploymentFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": dial tcp 10.96.38.90:443: connect: connection refused` + errConversionWebhookCertificateFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority` +) + func TestCmapiChecker(t *testing.T) { tests := map[string]testT{ "check API without errors": { - newError: nil, createError: nil, - expectedError: "", + expectedSimpleError: "", + expectedVerboseError: "", }, - "check API server unreachable": { - newError: errors.New("while creating client: Get \"http://localhost:8080/api?timeout=32s\": dial tcp 127.0.0.1:8080: connect: connection refused"), - createError: nil, + "check API without CRDs installed 1": { + createError: errors.New(errCertManagerCRDsMapping), - expectedError: ErrAPIServerUnreachable.Error(), + expectedSimpleError: ErrCertManagerCRDsNotFound.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrCertManagerCRDsNotFound.Error(), errCertManagerCRDsMapping), }, - "check API without CRDs installed": { - newError: nil, - createError: errors.New("error finding the scope of the object: failed to get restmapping: no matches for kind \"Certificate\" in group \"cert-manager.io\""), + "check API without CRDs installed 2": { + createError: errors.New(errCertManagerCRDsNotFound), - expectedError: ErrCertManagerCRDsNotFound.Error(), + expectedSimpleError: ErrCertManagerCRDsNotFound.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrCertManagerCRDsNotFound.Error(), errCertManagerCRDsNotFound), }, - "check API with webhook service not ready": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": service \"cert-manager-webhook\" not found"), - expectedError: ErrCertManagerAPIEndpointsNotEstablished.Error(), - }, - "check API with webhook pod not accepting connections": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": dial tcp 10.96.38.90:443: connect: connection refused"), + "check API with mutating webhook service not ready": { + createError: errors.New(errMutatingWebhookServiceFailure), - expectedError: ErrWebhookConnectionFailure.Error(), + expectedSimpleError: ErrWebhookServiceFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookServiceFailure.Error(), errMutatingWebhookServiceFailure), }, + "check API with conversion webhook service not ready": { + createError: errors.New(errConversionWebhookServiceFailure), + + expectedSimpleError: ErrWebhookServiceFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookServiceFailure.Error(), errConversionWebhookServiceFailure), + }, + + "check API with mutating webhook pod not accepting connections": { + createError: errors.New(errMutatingWebhookDeploymentFailure), + + expectedSimpleError: ErrWebhookDeploymentFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookDeploymentFailure.Error(), errMutatingWebhookDeploymentFailure), + }, + "check API with conversion webhook pod not accepting connections": { + createError: errors.New(errConversionWebhookDeploymentFailure), + + expectedSimpleError: ErrWebhookDeploymentFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookDeploymentFailure.Error(), errConversionWebhookDeploymentFailure), + }, + "check API with webhook certificate not updated in mutation webhook resource definitions": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": x509: certificate signed by unknown authority (possibly because of \"x509: ECDSA verification failure\" while trying to verify candidate authority certificate \"cert-manager-webhook-ca\""), + createError: errors.New(errMutatingWebhookCertificateFailure), - expectedError: ErrWebhookCertificateFailure.Error(), + expectedSimpleError: ErrWebhookCertificateFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookCertificateFailure.Error(), errMutatingWebhookCertificateFailure), }, "check API with webhook certificate not updated in conversion webhook resource definitions": { - newError: nil, - createError: errors.New("conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post \"https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s\": x509: certificate signed by unknown authority"), + createError: errors.New(errConversionWebhookCertificateFailure), - expectedError: ErrWebhookCertificateFailure.Error(), + expectedSimpleError: ErrWebhookCertificateFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookCertificateFailure.Error(), errConversionWebhookCertificateFailure), + }, + "unexpected error": { + createError: errors.New("unexpected error"), + + expectedSimpleError: "", + expectedVerboseError: "unexpected error", }, } @@ -119,26 +147,41 @@ func TestCmapiChecker(t *testing.T) { } type testT struct { - newError error createError error - expectedError string + expectedSimpleError string + expectedVerboseError string } func runTest(t *testing.T, test testT) { - errorClient, checker, _ := newFakeCmapiChecker() + errorClient, checker, err := newFakeCmapiChecker() + if err != nil { + t.Error(err) + } - errorClient.newError = test.newError errorClient.createError = test.createError - err := checker.Check(context.TODO()) + var unwrappedErr error + err = checker.Check(context.TODO()) if err != nil { - if err.Error() != test.expectedError { - t.Errorf("error differs from expected error:\n%s\n vs \n%s", err.Error(), test.expectedError) + if err.Error() != test.expectedVerboseError { + t.Errorf("error differs from expected error:\n%s\n vs \n%s", err.Error(), test.expectedVerboseError) + } + + unwrappedErr = errors.Unwrap(err) + } else { + if test.expectedVerboseError != "" { + t.Errorf("expected error did not occure:\n%s", test.expectedVerboseError) + } + } + + if unwrappedErr != nil { + if unwrappedErr.Error() != test.expectedSimpleError { + t.Errorf("simple error differs from expected error:\n%s\n vs \n%s", unwrappedErr.Error(), test.expectedSimpleError) } } else { - if test.expectedError != "" { - t.Errorf("expected error did not occure:\n%s", test.expectedError) + if test.expectedSimpleError != "" { + t.Errorf("expected simple error did not occure:\n%s", test.expectedSimpleError) } } }