diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index aa981740f..c8de18796 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -57,6 +57,11 @@ func warning(format string, v ...interface{}) { fmt.Fprintf(os.Stderr, format, v...) } +// hookOutputWriter provides the writer for writing hook logs. +func hookOutputWriter(_, _, _ string) io.Writer { + return log.Writer() +} + func main() { // Setting the name of the app for managedFields in the Kubernetes client. // It is set here to the full name of "helm" so that renaming of helm to @@ -80,6 +85,7 @@ func main() { if helmDriver == "memory" { loadReleasesInMemory(actionConfig) } + actionConfig.SetHookOutputFunc(hookOutputWriter) }) if err := cmd.Execute(); err != nil { diff --git a/pkg/action/action.go b/pkg/action/action.go index fef4a91f5..6efc6c2ee 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -19,6 +19,7 @@ package action import ( "bytes" "fmt" + "io" "os" "path" "path/filepath" @@ -95,6 +96,9 @@ type Configuration struct { Capabilities *chartutil.Capabilities Log func(string, ...interface{}) + + // HookOutputFunc called with container name and returns and expects writer that will receive the log output. + HookOutputFunc func(namespace, pod, container string) io.Writer } // renderResources renders the templates in a chart @@ -122,7 +126,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu var err2 error // A `helm template` should not talk to the remote cluster. However, commands with the flag - //`--dry-run` with the value of `false`, `none`, or `server` should try to interact with the cluster. + // `--dry-run` with the value of `false`, `none`, or `server` should try to interact with the cluster. // It may break in interesting and exotic ways because other data (e.g. discovery) is mocked. if interactWithRemote && cfg.RESTClientGetter != nil { restConfig, err := cfg.RESTClientGetter.ToRESTConfig() @@ -422,6 +426,12 @@ func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namesp cfg.KubeClient = kc cfg.Releases = store cfg.Log = log + cfg.HookOutputFunc = func(_, _, _ string) io.Writer { return io.Discard } return nil } + +// SetHookOutputFunc sets the HookOutputFunc on the Configuration. +func (cfg *Configuration) SetHookOutputFunc(hookOutputFunc func(_, _, _ string) io.Writer) { + cfg.HookOutputFunc = hookOutputFunc +} diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 8545b1edc..aa5d589f8 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -111,6 +111,14 @@ type chartOptions struct { type chartOption func(*chartOptions) func buildChart(opts ...chartOption) *chart.Chart { + defaultTemplates := []*chart.File{ + {Name: "templates/hello", Data: []byte("hello: world")}, + {Name: "templates/hooks", Data: []byte(manifestWithHook)}, + } + return buildChartWithTemplates(defaultTemplates, opts...) +} + +func buildChartWithTemplates(templates []*chart.File, opts ...chartOption) *chart.Chart { c := &chartOptions{ Chart: &chart.Chart{ // TODO: This should be more complete. @@ -119,18 +127,13 @@ func buildChart(opts ...chartOption) *chart.Chart { Name: "hello", Version: "0.1.0", }, - // This adds a basic template and hooks. - Templates: []*chart.File{ - {Name: "templates/hello", Data: []byte("hello: world")}, - {Name: "templates/hooks", Data: []byte(manifestWithHook)}, - }, + Templates: templates, }, } for _, opt := range opts { opt(c) } - return c.Chart } diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index ecca1d997..b6c505807 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -17,12 +17,19 @@ package action import ( "bytes" + "fmt" + "log" + "slices" "sort" "time" + "helm.sh/helm/v4/pkg/kube" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/pkg/errors" + "gopkg.in/yaml.v3" - "helm.sh/helm/v4/pkg/kube" "helm.sh/helm/v4/pkg/release" helmtime "helm.sh/helm/v4/pkg/time" ) @@ -87,10 +94,16 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, // Mark hook as succeeded or failed if err != nil { h.LastRun.Phase = release.HookPhaseFailed + // If a hook is failed, check the annotation of the hook to determine if we should copy the logs client side + if errOutputting := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnFailed); errOutputting != nil { + // We log the error here as we want to propagate the hook failure upwards to the release object. + log.Printf("error outputting logs for hook failure: %v", errOutputting) + } // If a hook is failed, check the annotation of the hook to determine whether the hook should be deleted // under failed condition. If so, then clear the corresponding resource object in the hook - if err := cfg.deleteHookByPolicy(h, release.HookFailed, timeout); err != nil { - return err + if errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed, timeout); errDeleting != nil { + // We log the error here as we want to propagate the hook failure upwards to the release object. + log.Printf("error deleting the hook resource on hook failure: %v", errDeleting) } return err } @@ -98,9 +111,13 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, } // If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted - // under succeeded condition. If so, then clear the corresponding resource object in each hook + // or output should be logged under succeeded condition. If so, then clear the corresponding resource object in each hook for i := len(executingHooks) - 1; i >= 0; i-- { h := executingHooks[i] + if err := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnSucceeded); err != nil { + // We log here as we still want to attempt hook resource deletion even if output logging fails. + log.Printf("error outputting logs for hook failure: %v", err) + } if err := cfg.deleteHookByPolicy(h, release.HookSucceeded, timeout); err != nil { return err } @@ -138,7 +155,7 @@ func (cfg *Configuration) deleteHookByPolicy(h *release.Hook, policy release.Hoo return errors.New(joinErrors(errs)) } - //wait for resources until they are deleted to avoid conflicts + // wait for resources until they are deleted to avoid conflicts if kubeClient, ok := cfg.KubeClient.(kube.InterfaceExt); ok { if err := kubeClient.WaitForDelete(resources, timeout); err != nil { return err @@ -158,3 +175,57 @@ func hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool } return false } + +// outputLogsByPolicy outputs a pods logs if the hook policy instructs it to +func (cfg *Configuration) outputLogsByPolicy(h *release.Hook, releaseNamespace string, policy release.HookOutputLogPolicy) error { + if !hookHasOutputLogPolicy(h, policy) { + return nil + } + namespace, err := cfg.deriveNamespace(h, releaseNamespace) + if err != nil { + return err + } + switch h.Kind { + case "Job": + return cfg.outputContainerLogsForListOptions(namespace, metav1.ListOptions{LabelSelector: fmt.Sprintf("job-name=%s", h.Name)}) + case "Pod": + return cfg.outputContainerLogsForListOptions(namespace, metav1.ListOptions{FieldSelector: fmt.Sprintf("metadata.name=%s", h.Name)}) + default: + return nil + } +} + +func (cfg *Configuration) outputContainerLogsForListOptions(namespace string, listOptions metav1.ListOptions) error { + // TODO Helm 4: Remove this check when GetPodList and OutputContainerLogsForPodList are moved from InterfaceLogs to Interface + if kubeClient, ok := cfg.KubeClient.(kube.InterfaceLogs); ok { + podList, err := kubeClient.GetPodList(namespace, listOptions) + if err != nil { + return err + } + err = kubeClient.OutputContainerLogsForPodList(podList, namespace, cfg.HookOutputFunc) + return err + } + return nil +} + +func (cfg *Configuration) deriveNamespace(h *release.Hook, namespace string) (string, error) { + tmp := struct { + Metadata struct { + Namespace string + } + }{} + err := yaml.Unmarshal([]byte(h.Manifest), &tmp) + if err != nil { + return "", errors.Wrapf(err, "unable to parse metadata.namespace from kubernetes manifest for output logs hook %s", h.Path) + } + if tmp.Metadata.Namespace == "" { + return namespace, nil + } + return tmp.Metadata.Namespace, nil +} + +// hookHasOutputLogPolicy determines whether the defined hook output log policy matches the hook output log policies +// supported by helm. +func hookHasOutputLogPolicy(h *release.Hook, policy release.HookOutputLogPolicy) bool { + return slices.Contains(h.OutputLogPolicies, policy) +} diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go new file mode 100644 index 000000000..0f4a9be34 --- /dev/null +++ b/pkg/action/hooks_test.go @@ -0,0 +1,208 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "fmt" + "io" + "testing" + + "github.com/stretchr/testify/assert" + + "helm.sh/helm/v4/pkg/chart" + kubefake "helm.sh/helm/v4/pkg/kube/fake" + "helm.sh/helm/v4/pkg/release" +) + +func podManifestWithOutputLogs(hookDefinitions []release.HookOutputLogPolicy) string { + hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions) + return fmt.Sprintf(`kind: Pod +metadata: + name: finding-sharky, + annotations: + "helm.sh/hook": pre-install + "helm.sh/hook-output-log-policy": %s +spec: + containers: + - name: sharky-test + image: fake-image + cmd: fake-command`, hookDefinitionString) +} + +func podManifestWithOutputLogWithNamespace(hookDefinitions []release.HookOutputLogPolicy) string { + hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions) + return fmt.Sprintf(`kind: Pod +metadata: + name: finding-george + namespace: sneaky-namespace + annotations: + "helm.sh/hook": pre-install + "helm.sh/hook-output-log-policy": %s +spec: + containers: + - name: george-test + image: fake-image + cmd: fake-command`, hookDefinitionString) +} + +func jobManifestWithOutputLog(hookDefinitions []release.HookOutputLogPolicy) string { + hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions) + return fmt.Sprintf(`kind: Job +apiVersion: batch/v1 +metadata: + name: losing-religion + annotations: + "helm.sh/hook": pre-install + "helm.sh/hook-output-log-policy": %s +spec: + completions: 1 + parallelism: 1 + activeDeadlineSeconds: 30 + template: + spec: + containers: + - name: religion-container + image: religion-image + cmd: religion-command`, hookDefinitionString) +} + +func jobManifestWithOutputLogWithNamespace(hookDefinitions []release.HookOutputLogPolicy) string { + hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions) + return fmt.Sprintf(`kind: Job +apiVersion: batch/v1 +metadata: + name: losing-religion + namespace: rem-namespace + annotations: + "helm.sh/hook": pre-install + "helm.sh/hook-output-log-policy": %s +spec: + completions: 1 + parallelism: 1 + activeDeadlineSeconds: 30 + template: + spec: + containers: + - name: religion-container + image: religion-image + cmd: religion-command`, hookDefinitionString) +} + +func convertHooksToCommaSeparated(hookDefinitions []release.HookOutputLogPolicy) string { + var commaSeparated string + for i, policy := range hookDefinitions { + if i+1 == len(hookDefinitions) { + commaSeparated += policy.String() + } else { + commaSeparated += policy.String() + "," + } + } + return commaSeparated +} + +func TestInstallRelease_HookOutputLogsOnFailure(t *testing.T) { + // Should output on failure with expected namespace if hook-failed is set + runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "sneaky-namespace", true) + runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "rem-namespace", true) + + // Should not output on failure with expected namespace if hook-succeed is set + runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false) + runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false) + runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false) + runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false) +} + +func TestInstallRelease_HookOutputLogsOnSuccess(t *testing.T) { + // Should output on success with expected namespace if hook-succeeded is set + runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "spaced", true) + runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "sneaky-namespace", true) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "spaced", true) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "rem-namespace", true) + + // Should not output on success if hook-failed is set + runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false) + runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false) +} + +func TestInstallRelease_HooksOutputLogsOnSuccessAndFailure(t *testing.T) { + // Should output on success with expected namespace if hook-succeeded and hook-failed is set + runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "sneaky-namespace", true) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "rem-namespace", true) + + // Should output on failure if hook-succeeded and hook-failed is set + runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "sneaky-namespace", true) + runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true) + runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "rem-namespace", true) +} + +func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) { + var expectedOutput string + if shouldOutput { + expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace) + } + is := assert.New(t) + instAction := installAction(t) + instAction.ReleaseName = "failed-hooks" + outBuffer := &bytes.Buffer{} + instAction.cfg.KubeClient = &kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} + + templates := []*chart.File{ + {Name: "templates/hello", Data: []byte("hello: world")}, + {Name: "templates/hooks", Data: []byte(manifest)}, + } + vals := map[string]interface{}{} + + res, err := instAction.Run(buildChartWithTemplates(templates), vals) + is.NoError(err) + is.Equal(expectedOutput, outBuffer.String()) + is.Equal(release.StatusDeployed, res.Info.Status) +} + +func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) { + var expectedOutput string + if shouldOutput { + expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace) + } + is := assert.New(t) + instAction := installAction(t) + instAction.ReleaseName = "failed-hooks" + failingClient := instAction.cfg.KubeClient.(*kubefake.FailingKubeClient) + failingClient.WatchUntilReadyError = fmt.Errorf("failed watch") + instAction.cfg.KubeClient = failingClient + outBuffer := &bytes.Buffer{} + failingClient.PrintingKubeClient = kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} + + templates := []*chart.File{ + {Name: "templates/hello", Data: []byte("hello: world")}, + {Name: "templates/hooks", Data: []byte(manifest)}, + } + vals := map[string]interface{}{} + + res, err := instAction.Run(buildChartWithTemplates(templates), vals) + is.Error(err) + is.Contains(res.Info.Description, "failed pre-install") + is.Equal(expectedOutput, outBuffer.String()) + is.Equal(release.StatusFailed, res.Info.Status) +} diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 171622e46..9d90accc5 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "bytes" "context" "fmt" "io" @@ -354,11 +355,14 @@ func TestInstallRelease_FailedHooks(t *testing.T) { failer := instAction.cfg.KubeClient.(*kubefake.FailingKubeClient) failer.WatchUntilReadyError = fmt.Errorf("Failed watch") instAction.cfg.KubeClient = failer + outBuffer := &bytes.Buffer{} + failer.PrintingKubeClient = kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} vals := map[string]interface{}{} res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(res.Info.Description, "failed post-install") + is.Equal("", outBuffer.String()) is.Equal(release.StatusFailed, res.Info.Status) } diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 0b84f5219..fd111c647 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -83,7 +83,7 @@ type Client struct { // Namespace allows to bypass the kubeconfig file for the choice of the namespace Namespace string - kubeClient *kubernetes.Clientset + kubeClient kubernetes.Interface } func init() { @@ -111,7 +111,7 @@ func New(getter genericclioptions.RESTClientGetter) *Client { var nopLogger = func(_ string, _ ...interface{}) {} // getKubeClient get or create a new KubernetesClientSet -func (c *Client) getKubeClient() (*kubernetes.Clientset, error) { +func (c *Client) getKubeClient() (kubernetes.Interface, error) { var err error if c.kubeClient == nil { c.kubeClient, err = c.Factory.KubernetesClientSet() @@ -131,7 +131,7 @@ func (c *Client) IsReachable() error { if err != nil { return errors.Wrap(err, "Kubernetes cluster unreachable") } - if _, err := client.ServerVersion(); err != nil { + if _, err := client.Discovery().ServerVersion(); err != nil { return errors.Wrap(err, "Kubernetes cluster unreachable") } return nil @@ -812,6 +812,48 @@ func (c *Client) waitForPodSuccess(obj runtime.Object, name string) (bool, error return false, nil } +// GetPodList uses the kubernetes interface to get the list of pods filtered by listOptions +func (c *Client) GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error) { + podList, err := c.kubeClient.CoreV1().Pods(namespace).List(context.Background(), listOptions) + if err != nil { + return nil, fmt.Errorf("failed to get pod list with options: %+v with error: %v", listOptions, err) + } + return podList, nil +} + +// OutputContainerLogsForPodList is a helper that outputs logs for a list of pods +func (c *Client) OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(namespace, pod, container string) io.Writer) error { + for _, pod := range podList.Items { + for _, container := range pod.Spec.Containers { + options := &v1.PodLogOptions{ + Container: container.Name, + } + request := c.kubeClient.CoreV1().Pods(namespace).GetLogs(pod.Name, options) + err2 := copyRequestStreamToWriter(request, pod.Name, container.Name, writerFunc(namespace, pod.Name, container.Name)) + if err2 != nil { + return err2 + } + } + } + return nil +} + +func copyRequestStreamToWriter(request *rest.Request, podName, containerName string, writer io.Writer) error { + readCloser, err := request.Stream(context.Background()) + if err != nil { + return errors.Errorf("Failed to stream pod logs for pod: %s, container: %s", podName, containerName) + } + defer readCloser.Close() + _, err = io.Copy(writer, readCloser) + if err != nil { + return errors.Errorf("Failed to copy IO from logs for pod: %s, container: %s", podName, containerName) + } + if err != nil { + return errors.Errorf("Failed to close reader for pod: %s, container: %s", podName, containerName) + } + return nil +} + // scrubValidationError removes kubectl info from the message. func scrubValidationError(err error) error { if err == nil { diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index f2d6bcb59..ff1335f0f 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -24,10 +24,13 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" + k8sfake "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest/fake" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" @@ -682,6 +685,39 @@ func TestReal(t *testing.T) { } } +func TestGetPodList(t *testing.T) { + + namespace := "some-namespace" + names := []string{"dave", "jimmy"} + var responsePodList v1.PodList + for _, name := range names { + responsePodList.Items = append(responsePodList.Items, newPodWithStatus(name, v1.PodStatus{}, namespace)) + } + + kubeClient := k8sfake.NewSimpleClientset(&responsePodList) + c := Client{Namespace: namespace, kubeClient: kubeClient} + + podList, err := c.GetPodList(namespace, metav1.ListOptions{}) + clientAssertions := assert.New(t) + clientAssertions.NoError(err) + clientAssertions.Equal(&responsePodList, podList) + +} + +func TestOutputContainerLogsForPodList(t *testing.T) { + namespace := "some-namespace" + somePodList := newPodList("jimmy", "three", "structs") + + kubeClient := k8sfake.NewSimpleClientset(&somePodList) + c := Client{Namespace: namespace, kubeClient: kubeClient} + outBuffer := &bytes.Buffer{} + outBufferFunc := func(_, _, _ string) io.Writer { return outBuffer } + err := c.OutputContainerLogsForPodList(&somePodList, namespace, outBufferFunc) + clientAssertions := assert.New(t) + clientAssertions.NoError(err) + clientAssertions.Equal("fake logsfake logsfake logs", outBuffer.String()) +} + const testServiceManifest = ` kind: Service apiVersion: v1 diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index 0b957d725..dcce9a3be 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -17,10 +17,12 @@ limitations under the License. package fake import ( + "fmt" "io" "strings" "time" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" @@ -31,7 +33,8 @@ import ( // PrintingKubeClient implements KubeClient, but simply prints the reader to // the given output. type PrintingKubeClient struct { - Out io.Writer + Out io.Writer + LogOutput io.Writer } // IsReachable checks if the cluster is reachable @@ -110,6 +113,22 @@ func (p *PrintingKubeClient) BuildTable(_ io.Reader, _ bool) (kube.ResourceList, return []*resource.Info{}, nil } +// WaitAndGetCompletedPodPhase implements KubeClient WaitAndGetCompletedPodPhase. +func (p *PrintingKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Duration) (v1.PodPhase, error) { + return v1.PodSucceeded, nil +} + +// GetPodList implements KubeClient GetPodList. +func (p *PrintingKubeClient) GetPodList(_ string, _ metav1.ListOptions) (*v1.PodList, error) { + return &v1.PodList{}, nil +} + +// OutputContainerLogsForPodList implements KubeClient OutputContainerLogsForPodList. +func (p *PrintingKubeClient) OutputContainerLogsForPodList(_ *v1.PodList, someNamespace string, _ func(namespace, pod, container string) io.Writer) error { + _, err := io.Copy(p.LogOutput, strings.NewReader(fmt.Sprintf("attempted to output logs for namespace: %s", someNamespace))) + return err +} + // DeleteWithPropagationPolicy implements KubeClient delete. // // It only prints out the content to be deleted. diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 7dc7ad8bc..c9776cacd 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -20,6 +20,7 @@ import ( "io" "time" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -67,7 +68,7 @@ type Interface interface { IsReachable() error } -// InterfaceExt is introduced to avoid breaking backwards compatibility for Interface implementers. +// InterfaceExt was introduced to avoid breaking backwards compatibility for Interface implementers. // // TODO Helm 4: Remove InterfaceExt and integrate its method(s) into the Interface. type InterfaceExt interface { @@ -75,11 +76,22 @@ type InterfaceExt interface { WaitForDelete(resources ResourceList, timeout time.Duration) error } +// InterfaceLogs was introduced to avoid breaking backwards compatibility for Interface implementers. +// +// TODO Helm 4: Remove InterfaceLogs and integrate its method(s) into the Interface. +type InterfaceLogs interface { + // GetPodList list all pods that match the specified listOptions + GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error) + + // OutputContainerLogsForPodList output the logs for a pod list + OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(namespace, pod, container string) io.Writer) error +} + // InterfaceDeletionPropagation is introduced to avoid breaking backwards compatibility for Interface implementers. // // TODO Helm 4: Remove InterfaceDeletionPropagation and integrate its method(s) into the Interface. type InterfaceDeletionPropagation interface { - // Delete destroys one or more resources. The deletion propagation is handled as per the given deletion propagation value. + // DeleteWithPropagationPolicy destroys one or more resources. The deletion propagation is handled as per the given deletion propagation value. DeleteWithPropagationPolicy(resources ResourceList, policy metav1.DeletionPropagation) (*Result, []error) } @@ -107,5 +119,6 @@ type InterfaceResources interface { var _ Interface = (*Client)(nil) var _ InterfaceExt = (*Client)(nil) +var _ InterfaceLogs = (*Client)(nil) var _ InterfaceDeletionPropagation = (*Client)(nil) var _ InterfaceResources = (*Client)(nil) diff --git a/pkg/release/hook.go b/pkg/release/hook.go index e2cdc0eb8..5ff61fdaa 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -50,6 +50,17 @@ const ( func (x HookDeletePolicy) String() string { return string(x) } +// HookOutputLogPolicy specifies the hook output log policy +type HookOutputLogPolicy string + +// Hook output log policy types +const ( + HookOutputOnSucceeded HookOutputLogPolicy = "hook-succeeded" + HookOutputOnFailed HookOutputLogPolicy = "hook-failed" +) + +func (x HookOutputLogPolicy) String() string { return string(x) } + // HookAnnotation is the label name for a hook const HookAnnotation = "helm.sh/hook" @@ -59,6 +70,9 @@ const HookWeightAnnotation = "helm.sh/hook-weight" // HookDeleteAnnotation is the label name for the delete policy for a hook const HookDeleteAnnotation = "helm.sh/hook-delete-policy" +// HookOutputLogAnnotation is the label name for the output log policy for a hook +const HookOutputLogAnnotation = "helm.sh/hook-output-log-policy" + // Hook defines a hook object. type Hook struct { Name string `json:"name,omitempty"` @@ -76,6 +90,8 @@ type Hook struct { Weight int `json:"weight,omitempty"` // DeletePolicies are the policies that indicate when to delete the hook DeletePolicies []HookDeletePolicy `json:"delete_policies,omitempty"` + // OutputLogPolicies defines whether we should copy hook logs back to main process + OutputLogPolicies []HookOutputLogPolicy `json:"output_log_policies,omitempty"` } // A HookExecution records the result for the last execution of a hook for a given release. diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index c7d92b184..2d9a14bf1 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -128,6 +128,14 @@ func SortManifests(files map[string]string, _ chartutil.VersionSet, ordering Kin // metadata: // annotations: // helm.sh/hook-delete-policy: hook-succeeded +// +// To determine the policy to output logs of the hook (for Pod and Job only), it looks for a YAML structure like this: +// +// kind: Pod +// apiVersion: v1 +// metadata: +// annotations: +// helm.sh/hook-output-log-policy: hook-succeeded,hook-failed func (file *manifestFile) sort(result *result) error { // Go through manifests in order found in file (function `SplitManifests` creates integer-sortable keys) var sortedEntryKeys []string @@ -166,13 +174,14 @@ func (file *manifestFile) sort(result *result) error { hw := calculateHookWeight(entry) h := &release.Hook{ - Name: entry.Metadata.Name, - Kind: entry.Kind, - Path: file.path, - Manifest: m, - Events: []release.HookEvent{}, - Weight: hw, - DeletePolicies: []release.HookDeletePolicy{}, + Name: entry.Metadata.Name, + Kind: entry.Kind, + Path: file.path, + Manifest: m, + Events: []release.HookEvent{}, + Weight: hw, + DeletePolicies: []release.HookDeletePolicy{}, + OutputLogPolicies: []release.HookOutputLogPolicy{}, } isUnknownHook := false @@ -196,6 +205,10 @@ func (file *manifestFile) sort(result *result) error { operateAnnotationValues(entry, release.HookDeleteAnnotation, func(value string) { h.DeletePolicies = append(h.DeletePolicies, release.HookDeletePolicy(value)) }) + + operateAnnotationValues(entry, release.HookOutputLogAnnotation, func(value string) { + h.OutputLogPolicies = append(h.OutputLogPolicies, release.HookOutputLogPolicy(value)) + }) } return nil