From cc3b8dc84f9ad2a5256ddfd9478d7c4c34e786c2 Mon Sep 17 00:00:00 2001 From: Chris Berry Date: Fri, 7 Jan 2022 13:37:19 +0000 Subject: [PATCH 1/3] Add hook annotations to output pod logs to client on success and fail Signed-off-by: Chris Berry --- pkg/action/action_test.go | 15 ++- pkg/action/hooks.go | 85 +++++++++++- pkg/action/hooks_test.go | 208 +++++++++++++++++++++++++++++ pkg/action/install_test.go | 4 + pkg/kube/client.go | 50 ++++++- pkg/kube/client_test.go | 35 +++++ pkg/kube/fake/printer.go | 17 ++- pkg/kube/interface.go | 8 ++ pkg/release/hook.go | 16 +++ pkg/releaseutil/manifest_sorter.go | 26 +++- 10 files changed, 444 insertions(+), 20 deletions(-) create mode 100644 pkg/action/hooks_test.go diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index f8bdff3b7fc..7850e90d677 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -117,6 +117,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. @@ -125,18 +133,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 40c1ffdb6dd..0f4e6d36ed0 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -17,9 +17,17 @@ package action import ( "bytes" + "fmt" + "log" "sort" "time" + "helm.sh/helm/v3/pkg/kube" + + "helm.sh/helm/v3/pkg/chartutil" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/pkg/errors" "helm.sh/helm/v3/pkg/release" @@ -86,10 +94,18 @@ 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); err != nil { - return err + if errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed); errDeleting != nil { + // We log the error here as we want to propagate the hook failure upwards to the release object. + // This is a change in behaviour as the edge case previously would lose the hook error and only + // raise the delete hook error. + log.Printf("error the hook resource on hook failure: %v", errDeleting) } return err } @@ -97,8 +113,12 @@ 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 _, h := range executingHooks { + 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); err != nil { return err } @@ -149,3 +169,62 @@ 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) { + 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 + } + } + 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 InterfaceExt to Interface + if kubeClient, ok := cfg.KubeClient.(kube.InterfaceExt); ok { + podList, err := kubeClient.GetPodList(namespace, listOptions) + if err != nil { + return err + } + err = kubeClient.OutputContainerLogsForPodList(podList, namespace, log.Writer()) + return err + } + return nil +} + +func (cfg *Configuration) deriveNamespace(h *release.Hook, namespace string) (string, error) { + values, err := chartutil.ReadValues([]byte(h.Manifest)) + if err != nil { + return "", errors.Wrapf(err, "unable to parse kubernetes manifest for output logs hook %s", h.Path) + } + value, err := values.PathValue("metadata.namespace") + switch err.(type) { + case nil: + return value.(string), nil + case chartutil.ErrNoValue: + return namespace, nil + default: + return "", errors.Wrapf(err, "unable to parse path of metadata.namespace in yaml for output logs hook %s", h.Path) + } +} + +// 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 { + for _, v := range h.OutputLogPolicies { + if policy == v { + return true + } + } + return false +} diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go new file mode 100644 index 00000000000..25a28f60ff6 --- /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/ioutil" + "testing" + + "github.com/stretchr/testify/assert" + + "helm.sh/helm/v3/pkg/chart" + kubefake "helm.sh/helm/v3/pkg/kube/fake" + "helm.sh/helm/v3/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: ioutil.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: ioutil.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 b1844b2ce02..be1944b7d2a 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/ioutil" @@ -302,11 +303,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: ioutil.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 cc38243ac40..9db024e7443 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -27,6 +27,8 @@ import ( "sync" "time" + "k8s.io/client-go/rest" + jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" batch "k8s.io/api/batch/v1" @@ -68,7 +70,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 } var addToScheme sync.Once @@ -97,7 +99,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() @@ -117,7 +119,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 @@ -620,6 +622,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, writer 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, writer) + 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 de5358aee87..3b591220b4a 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -24,10 +24,13 @@ import ( "strings" "testing" + "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" @@ -343,6 +346,38 @@ 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{} + err := c.OutputContainerLogsForPodList(&somePodList, namespace, outBuffer) + 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 1e8cf0066ca..033d636dd10 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -17,10 +17,13 @@ limitations under the License. package fake import ( + "fmt" "io" "strings" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/api/core/v1" "k8s.io/cli-runtime/pkg/resource" @@ -30,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 @@ -101,6 +105,17 @@ func (p *PrintingKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Durati 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, _ io.Writer) error { + _, err := io.Copy(p.LogOutput, strings.NewReader(fmt.Sprintf("attempted to output logs for namespace: %s", someNamespace))) + return err +} + func bufferize(resources kube.ResourceList) io.Reader { var builder strings.Builder for _, info := range resources { diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 299e34e953b..ca87669268e 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -20,6 +20,8 @@ import ( "io" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/api/core/v1" ) @@ -76,6 +78,12 @@ type Interface interface { type InterfaceExt interface { // WaitForDelete wait up to the given timeout for the specified resources to be deleted. WaitForDelete(resources ResourceList, timeout time.Duration) error + + // 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, writer io.Writer) error } var _ Interface = (*Client)(nil) diff --git a/pkg/release/hook.go b/pkg/release/hook.go index cb995558225..425074ac1d4 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 e834145000c..6b9d7220566 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -130,6 +130,13 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering // 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 @@ -168,13 +175,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 @@ -198,6 +206,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 From ec41082abb023117cce31033d0323952c85985e5 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 3 May 2023 10:35:25 +0100 Subject: [PATCH 2/3] Tidy up imports Signed-off-by: Chris --- pkg/action/hooks_test.go | 6 +++--- pkg/action/install_test.go | 2 +- pkg/kube/client.go | 2 -- pkg/kube/fake/printer.go | 2 -- pkg/kube/interface.go | 2 -- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go index 25a28f60ff6..76de9e5055f 100644 --- a/pkg/action/hooks_test.go +++ b/pkg/action/hooks_test.go @@ -19,7 +19,7 @@ package action import ( "bytes" "fmt" - "io/ioutil" + "io" "testing" "github.com/stretchr/testify/assert" @@ -166,7 +166,7 @@ func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace str instAction := installAction(t) instAction.ReleaseName = "failed-hooks" outBuffer := &bytes.Buffer{} - instAction.cfg.KubeClient = &kubefake.PrintingKubeClient{Out: ioutil.Discard, LogOutput: outBuffer} + instAction.cfg.KubeClient = &kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} templates := []*chart.File{ {Name: "templates/hello", Data: []byte("hello: world")}, @@ -192,7 +192,7 @@ func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace str failingClient.WatchUntilReadyError = fmt.Errorf("failed watch") instAction.cfg.KubeClient = failingClient outBuffer := &bytes.Buffer{} - failingClient.PrintingKubeClient = kubefake.PrintingKubeClient{Out: ioutil.Discard, LogOutput: outBuffer} + failingClient.PrintingKubeClient = kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} templates := []*chart.File{ {Name: "templates/hello", Data: []byte("hello: world")}, diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 4ae41ed9956..b4765417d96 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -315,7 +315,7 @@ func TestInstallRelease_FailedHooks(t *testing.T) { failer.WatchUntilReadyError = fmt.Errorf("Failed watch") instAction.cfg.KubeClient = failer outBuffer := &bytes.Buffer{} - failer.PrintingKubeClient = kubefake.PrintingKubeClient{Out: ioutil.Discard, LogOutput: outBuffer} + failer.PrintingKubeClient = kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer} vals := map[string]interface{}{} res, err := instAction.Run(buildChart(), vals) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index b1c7d0fc1de..025cf633d1b 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -29,8 +29,6 @@ import ( "sync" "time" - "k8s.io/client-go/rest" - jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" batch "k8s.io/api/batch/v1" diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index c714074a69b..2c696b542a7 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -22,8 +22,6 @@ import ( "strings" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index b87fc31bc1e..9b01fc3770f 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -20,8 +20,6 @@ import ( "io" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" From 1d7566a5a838f1003933da146fdc20cfcce701a3 Mon Sep 17 00:00:00 2001 From: Ian Zink Date: Mon, 18 Sep 2023 16:51:15 -0500 Subject: [PATCH 3/3] Add configuration for hook log output --- pkg/action/action.go | 9 +++++++++ pkg/action/hooks.go | 3 ++- pkg/kube/client.go | 3 ++- pkg/kube/client_test.go | 2 +- pkg/kube/interface.go | 2 +- 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 016aec3f6e9..546e0ccef72 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" @@ -94,6 +95,9 @@ type Configuration struct { // Capabilities describes the capabilities of the Kubernetes cluster. Capabilities *chartutil.Capabilities + // Called with container name and returns and expects writer that will receive the log output + HookOutputFunc func(namespace string, pod string, container string) io.Writer + Log func(string, ...interface{}) } @@ -422,5 +426,10 @@ func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namesp cfg.Releases = store cfg.Log = log + cfg.HookOutputFunc = func(namespace string, pod string, container string) io.Writer { + fmt.Fprint(os.Stdout, fmt.Sprintf("Logs for pod: %s, container: %s:\n", pod, container)) + return os.Stdout + } + return nil } diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 0f4e6d36ed0..8a68b829a01 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -196,7 +196,8 @@ func (cfg *Configuration) outputContainerLogsForListOptions(namespace string, li if err != nil { return err } - err = kubeClient.OutputContainerLogsForPodList(podList, namespace, log.Writer()) + logWriterFunc := cfg.HookOutputFunc + err = kubeClient.OutputContainerLogsForPodList(podList, namespace, logWriterFunc) return err } return nil diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 025cf633d1b..31d29937de0 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -808,13 +808,14 @@ func (c *Client) GetPodList(namespace string, listOptions metav1.ListOptions) (* } // OutputContainerLogsForPodList is a helper that outputs logs for a list of pods -func (c *Client) OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writer io.Writer) error { +func (c *Client) OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(string, string, 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) + writer := writerFunc(pod.Namespace, pod.Name, container.Name) err2 := copyRequestStreamToWriter(request, pod.Name, container.Name, writer) if err2 != nil { return err2 diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 3a0cbed2ecd..bd10bae8da6 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -410,7 +410,7 @@ func TestOutputContainerLogsForPodList(t *testing.T) { kubeClient := k8sfake.NewSimpleClientset(&somePodList) c := Client{Namespace: namespace, kubeClient: kubeClient} outBuffer := &bytes.Buffer{} - err := c.OutputContainerLogsForPodList(&somePodList, namespace, outBuffer) + err := c.OutputContainerLogsForPodList(&somePodList, namespace, func(string, string, string) io.Writer { return outBuffer }) clientAssertions := assert.New(t) clientAssertions.NoError(err) clientAssertions.Equal("fake logsfake logsfake logs", outBuffer.String()) diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 9b01fc3770f..6b038d68932 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -83,7 +83,7 @@ type InterfaceExt interface { GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error) // OutputContainerLogsForPodList output the logs for a pod list - OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writer io.Writer) error + OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(namespace string, pod string, container string) io.Writer) error } // InterfaceDeletionPropagation is introduced to avoid breaking backwards compatibility for Interface implementers.