From 11f4e4968e9dde5a918a721b6783f235ca8ffcfa Mon Sep 17 00:00:00 2001 From: Cole Snodgrass Date: Thu, 12 Dec 2024 17:09:06 -0800 Subject: [PATCH] remove helm secrets if previous attempt is stuck --- internal/cmd/local/k8s/client.go | 39 +++--- internal/cmd/local/k8s/k8stest/k8stest.go | 9 ++ internal/cmd/local/local/install.go | 61 +++++---- internal/cmd/local/local/install_test.go | 154 ++++++++++++++++++++++ 4 files changed, 219 insertions(+), 44 deletions(-) diff --git a/internal/cmd/local/k8s/client.go b/internal/cmd/local/k8s/client.go index 0358703..728f173 100644 --- a/internal/cmd/local/k8s/client.go +++ b/internal/cmd/local/k8s/client.go @@ -33,50 +33,40 @@ type Client interface { // This is a blocking call, it should only return once the deployment has completed. DeploymentRestart(ctx context.Context, namespace, name string) error - // IngressCreate creates an ingress in the given namespace + EventsWatch(ctx context.Context, namespace string) (watch.Interface, error) + IngressCreate(ctx context.Context, namespace string, ingress *networkingv1.Ingress) error - // IngressExists returns true if the ingress exists in the namespace, false otherwise. IngressExists(ctx context.Context, namespace string, ingress string) bool - // IngressUpdate updates an existing ingress in the given namespace IngressUpdate(ctx context.Context, namespace string, ingress *networkingv1.Ingress) error - // NamespaceCreate creates a namespace + LogsGet(ctx context.Context, namespace string, name string) (string, error) + NamespaceCreate(ctx context.Context, namespace string) error - // NamespaceExists returns true if the namespace exists, false otherwise NamespaceExists(ctx context.Context, namespace string) bool - // NamespaceDelete deletes the existing namespace NamespaceDelete(ctx context.Context, namespace string) error - // PersistentVolumeCreate creates a persistent volume PersistentVolumeCreate(ctx context.Context, namespace, name string) error - // PersistentVolumeExists returns true if the persistent volume exists, false otherwise PersistentVolumeExists(ctx context.Context, namespace, name string) bool - // PersistentVolumeDelete deletes the existing persistent volume PersistentVolumeDelete(ctx context.Context, namespace, name string) error - // PersistentVolumeClaimCreate creates a persistent volume claim PersistentVolumeClaimCreate(ctx context.Context, namespace, name, volumeName string) error - // PersistentVolumeClaimExists returns true if the persistent volume claim exists, false otherwise PersistentVolumeClaimExists(ctx context.Context, namespace, name, volumeName string) bool - // PersistentVolumeClaimDelete deletes the existing persistent volume claim PersistentVolumeClaimDelete(ctx context.Context, namespace, name, volumeName string) error - // SecretCreateOrUpdate will update or create the secret name with the payload of data in the specified namespace + PodList(ctx context.Context, namespace string) (*corev1.PodList, error) + SecretCreateOrUpdate(ctx context.Context, secret corev1.Secret) error - // SecretGet returns the secrets for the namespace and name + // SecretDeleteCollection deletes multiple secrets. + // Note this takes a `type` and not a `name`. All secrets matching this type will be removed. + SecretDeleteCollection(ctx context.Context, namespace, _type string) error SecretGet(ctx context.Context, namespace, name string) (*corev1.Secret, error) - // ServiceGet returns the service for the given namespace and name ServiceGet(ctx context.Context, namespace, name string) (*corev1.Service, error) + StreamPodLogs(ctx context.Context, namespace string, podName string, since time.Time) (io.ReadCloser, error) + // ServerVersionGet returns the kubernetes version. ServerVersionGet() (string, error) - - EventsWatch(ctx context.Context, namespace string) (watch.Interface, error) - - LogsGet(ctx context.Context, namespace string, name string) (string, error) - StreamPodLogs(ctx context.Context, namespace string, podName string, since time.Time) (io.ReadCloser, error) - PodList(ctx context.Context, namespace string) (*corev1.PodList, error) } var _ Client = (*DefaultK8sClient)(nil) @@ -289,6 +279,13 @@ func (d *DefaultK8sClient) SecretCreateOrUpdate(ctx context.Context, secret core return fmt.Errorf("unexpected error while handling the secret %s: %w", name, err) } +func (d *DefaultK8sClient) SecretDeleteCollection(ctx context.Context, namespace, _type string) error { + listOptions := metav1.ListOptions{ + FieldSelector: "type=" + _type, + } + return d.ClientSet.CoreV1().Secrets(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, listOptions) +} + func (d *DefaultK8sClient) SecretGet(ctx context.Context, namespace, name string) (*corev1.Secret, error) { secret, err := d.ClientSet.CoreV1().Secrets(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { diff --git a/internal/cmd/local/k8s/k8stest/k8stest.go b/internal/cmd/local/k8s/k8stest/k8stest.go index 8b4e0e8..cb6878b 100644 --- a/internal/cmd/local/k8s/k8stest/k8stest.go +++ b/internal/cmd/local/k8s/k8stest/k8stest.go @@ -30,6 +30,7 @@ type MockClient struct { FnPersistentVolumeClaimExists func(ctx context.Context, namespace, name, volumeName string) bool FnPersistentVolumeClaimDelete func(ctx context.Context, namespace, name, volumeName string) error FnSecretCreateOrUpdate func(ctx context.Context, secret corev1.Secret) error + FnSecretDeleteCollection func(ctx context.Context, namespace, _type string) error FnSecretGet func(ctx context.Context, namespace, name string) (*corev1.Secret, error) FnServerVersionGet func() (string, error) FnServiceGet func(ctx context.Context, namespace, name string) (*corev1.Service, error) @@ -146,6 +147,14 @@ func (m *MockClient) SecretGet(ctx context.Context, namespace, name string) (*co return nil, nil } +func (m *MockClient) SecretDeleteCollection(ctx context.Context, namespace, _type string) error { + if m.FnSecretDeleteCollection != nil { + return m.FnSecretDeleteCollection(ctx, namespace, _type) + } + + return nil +} + func (m *MockClient) ServiceGet(ctx context.Context, namespace, name string) (*corev1.Service, error) { return m.FnServiceGet(ctx, namespace, name) } diff --git a/internal/cmd/local/local/install.go b/internal/cmd/local/local/install.go index c3124a4..520d251 100644 --- a/internal/cmd/local/local/install.go +++ b/internal/cmd/local/local/install.go @@ -648,29 +648,44 @@ func (c *Command) handleChart( } } - pterm.Info.Println(fmt.Sprintf( - "Starting Helm Chart installation of '%s' (version: %s)", - req.chartName, helmChart.Metadata.Version, - )) - c.spinner.UpdateText(fmt.Sprintf( - "Installing '%s' (version: %s) Helm Chart (this may take several minutes)", - req.chartName, helmChart.Metadata.Version, - )) - helmRelease, err := c.helm.InstallOrUpgradeChart(ctx, &helmclient.ChartSpec{ - ReleaseName: req.chartRelease, - ChartName: req.chartLoc, - CreateNamespace: true, - Namespace: req.namespace, - Wait: true, - Timeout: 60 * time.Minute, - ValuesYaml: req.valuesYAML, - Version: req.chartVersion, - }, - &helmclient.GenericHelmOptions{}, - ) - if err != nil { - pterm.Error.Printfln("Failed to install %s Helm Chart", req.chartName) - return fmt.Errorf("unable to install helm: %w", err) + var helmRelease *release.Release + + // it's possible that an existing helm installation is stuck in a non-final state + // which this code will detect, attempt to clean up, and try again up to three times + for attemptCount := 0; attemptCount < 3; attemptCount++ { + pterm.Info.Println(fmt.Sprintf( + "Starting Helm Chart installation of '%s' (version: %s)", + req.chartName, helmChart.Metadata.Version, + )) + c.spinner.UpdateText(fmt.Sprintf( + "Installing '%s' (version: %s) Helm Chart (this may take several minutes)", + req.chartName, helmChart.Metadata.Version, + )) + + helmRelease, err = c.helm.InstallOrUpgradeChart(ctx, &helmclient.ChartSpec{ + ReleaseName: req.chartRelease, + ChartName: req.chartLoc, + CreateNamespace: true, + Namespace: req.namespace, + Wait: true, + Timeout: 60 * time.Minute, + ValuesYaml: req.valuesYAML, + Version: req.chartVersion, + }, + &helmclient.GenericHelmOptions{}, + ) + + if err != nil { + if strings.Contains(err.Error(), "another operation (install/upgrade/rollback) is in progress") { + if err := c.k8s.SecretDeleteCollection(ctx, common.AirbyteNamespace, "helm.sh/release.v1"); err != nil { + pterm.Debug.Println(fmt.Sprintf("unable to delete secrets helm.sh/release.v1: %s", err)) + } + continue + } + pterm.Error.Printfln("Failed to install %s Helm Chart", req.chartName) + return fmt.Errorf("unable to install helm: %w", err) + } + break } c.tel.Attr(fmt.Sprintf("helm_%s_release_version", req.name), strconv.Itoa(helmRelease.Version)) diff --git a/internal/cmd/local/local/install_test.go b/internal/cmd/local/local/install_test.go index 2998b6d..8acab97 100644 --- a/internal/cmd/local/local/install_test.go +++ b/internal/cmd/local/local/install_test.go @@ -172,6 +172,159 @@ func TestCommand_Install(t *testing.T) { } } +func TestCommand_InstallBadHelmState(t *testing.T) { + valuesYaml := mustReadFile(t, "testdata/test-edition.values.yaml") + expChartRepoCnt := 0 + expChartRepo := []struct { + name string + url string + }{ + {name: common.AirbyteRepoName, url: common.AirbyteRepoURL}, + {name: common.NginxRepoName, url: common.NginxRepoURL}, + } + + expChartCnt := 0 + expNginxValues, _ := helm.BuildNginxValues(9999) + expChart := []struct { + chart helmclient.ChartSpec + release release.Release + }{ + { + chart: helmclient.ChartSpec{ + ReleaseName: common.AirbyteChartRelease, + ChartName: testAirbyteChartLoc, + Namespace: common.AirbyteNamespace, + CreateNamespace: true, + Wait: true, + Timeout: 60 * time.Minute, + ValuesYaml: valuesYaml, + }, + release: release.Release{ + Chart: &chart.Chart{Metadata: &chart.Metadata{Version: "1.2.3.4"}}, + Name: common.AirbyteChartRelease, + Namespace: common.AirbyteNamespace, + Version: 0, + }, + }, + { + chart: helmclient.ChartSpec{ + ReleaseName: common.NginxChartRelease, + ChartName: common.NginxChartName, + Namespace: common.NginxNamespace, + CreateNamespace: true, + Wait: true, + Timeout: 60 * time.Minute, + ValuesYaml: expNginxValues, + }, + release: release.Release{ + Chart: &chart.Chart{Metadata: &chart.Metadata{Version: "4.3.2.1"}}, + Name: common.NginxChartRelease, + Namespace: common.NginxNamespace, + Version: 0, + }, + }, + } + + installCalled := false + helm := mockHelmClient{ + addOrUpdateChartRepo: func(entry repo.Entry) error { + if d := cmp.Diff(expChartRepo[expChartRepoCnt].name, entry.Name); d != "" { + t.Error("chart name mismatch", d) + } + if d := cmp.Diff(expChartRepo[expChartRepoCnt].url, entry.URL); d != "" { + t.Error("chart url mismatch", d) + } + + expChartRepoCnt++ + + return nil + }, + + getChart: func(name string, _ *action.ChartPathOptions) (*chart.Chart, string, error) { + switch { + case name == testAirbyteChartLoc: + return &chart.Chart{Metadata: &chart.Metadata{Version: "test.airbyte.version"}}, "", nil + case name == common.NginxChartName: + return &chart.Chart{Metadata: &chart.Metadata{Version: "test.nginx.version"}}, "", nil + default: + t.Error("unsupported chart name", name) + return nil, "", errors.New("unexpected chart name") + } + }, + + getRelease: func(name string) (*release.Release, error) { + switch { + case name == common.AirbyteChartRelease: + t.Error("should not have been called", name) + return nil, errors.New("should not have been called") + case name == common.NginxChartRelease: + return nil, errors.New("not found") + default: + t.Error("unsupported chart name", name) + return nil, errors.New("unexpected chart name") + } + }, + + installOrUpgradeChart: func(ctx context.Context, spec *helmclient.ChartSpec, opts *helmclient.GenericHelmOptions) (*release.Release, error) { + if d := cmp.Diff(&expChart[expChartCnt].chart, spec); d != "" { + t.Error("chart mismatch", d) + } + + if installCalled { + defer func() { expChartCnt++ }() + + return &expChart[expChartCnt].release, nil + } + + installCalled = true + return nil, errors.New("another operation (install/upgrade/rollback) is in progress") + }, + + uninstallReleaseByName: func(s string) error { + if d := cmp.Diff(expChart[expChartCnt].release.Name, s); d != "" { + t.Error("release mismatch", d) + } + + return nil + }, + } + + k8sClient := k8stest.MockClient{ + FnIngressExists: func(ctx context.Context, namespace string, ingress string) bool { + return false + }, + } + + tel := telemetry.MockClient{} + + httpClient := mockHTTP{do: func(req *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: 200}, nil + }} + + c, err := New( + k8s.TestProvider, + WithPortHTTP(portTest), + WithHelmClient(&helm), + WithK8sClient(&k8sClient), + WithTelemetryClient(&tel), + WithHTTPClient(&httpClient), + WithBrowserLauncher(func(url string) error { + return nil + }), + ) + if err != nil { + t.Fatal(err) + } + + installOpts := &InstallOpts{ + HelmValuesYaml: valuesYaml, + AirbyteChartLoc: testAirbyteChartLoc, + } + if err := c.Install(context.Background(), installOpts); err != nil { + t.Fatal(err) + } +} + func TestCommand_InstallError(t *testing.T) { testErr := errors.New("test error") valuesYaml := mustReadFile(t, "testdata/test-edition.values.yaml") @@ -234,6 +387,7 @@ func TestCommand_InstallError(t *testing.T) { } func mustReadFile(t *testing.T, name string) string { + t.Helper() b, err := os.ReadFile(name) if err != nil { t.Fatal(err)