Repository: helm/helm
Pull request #10309 (2021-11-03 10:52:24)

Add hook annotation to output hook logs to client on error
Signed-off-by: Chris Berry <CBE44@ukc02zxr8zmd6m.bskyb.com>

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:
- Implements https://github.com/helm/community/pull/301

Addresses #3481 and #2298 in outputting the logs of hook jobs and pods on failure.

Happy to add test coverage and documentation, just wanted early feedback of whether this approach would be acceptable.

**Special notes for your reviewer**:
* Using the existing c.Log(...) to leverage the --debug flag can't really work here. We could read the container logs through a buffer, but c.Log adds a newline, which would split the log lines.
* There doesn't seem to be a clear place to add tests for this, but we'll find somewhere if the general approach looks ok.

**If applicable**:
- [x] this PR contains documentation - https://github.com/helm/helm-www/pull/1242
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


arnonki: It would be nice if we will have additional hook for getting the hook job/pod logs without a condition (not only on error).
Would you consider?

Bez625: > It would be nice if we will have additional hook for getting the hook job/pod logs without a condition (not only on error). Would you consider?

Hey @arnonki, we might be able to change the `ShouldOutputLogs` field to work similar to `DeletePolicies` e.g. take a comma separated list of `hook-succeeded,hook-failed`, but to be honest we have been waiting on feedback from the maintainers on whether it is worth us proceeding with this approach at all. 

We don't really want to invest time and effort writing tests and documentation if the idea is going to be rejected. 

We have tried reaching out to them on the Kubernetes slack and have now sent a mail to the cncf.io mailing list. If you like you could contribute to that mailing list discussion on helm cncf list https://lists.cncf.io/g/cncf-helm.


bacongobbler: Hi @Bez625. A couple of quick notes.

I noticed no test coverage was provided with these changes. That makes it much more difficult for the maintainers to review as we don't know what's the expected behaviour of the code. Is it a bug or a feature? We strive for 80% test coverage.

I don't see any documentation surrounding how to test this feature ("if I do X, I should expect to see Y"), how users are expected to use this (how do I set this up?), and how it is intended to be used (and in some cases, what scenarios it is not intended for use).

Those two things alone make it incredibly difficult to gauge how to evaluate this feature. As it stands now all I see is some code without much context.

Addressing those concerns may help present your use case to the maintainers for review. By making it easier for us to review, we can provide quicker feedback on your proposal.

Hope that helps.
Bez625: > Hi @Bez625. A couple of quick notes.
> 
> I noticed no test coverage was provided with these changes. That makes it much more difficult for the maintainers to review as we don't know what's the expected behaviour of the code. Is it a bug or a feature? We strive for 80% test coverage.
> 
> I don't see any documentation surrounding how to test this feature ("if I do X, I should expect to see Y"), how users are expected to use this (how do I set this up?), and how it is intended to be used (and in some cases, what scenarios it is not intended for use).
> 
> Those two things alone make it incredibly difficult to evaluate this feature. As it stands, all I see is some code without much context.
> 
> Addressing those concerns may help present your use case to the maintainers for review. By making it easier for us to review, we can provide quicker feedback on your proposal.
> 
> Hope that helps.

Hi Matt,

Those are fair comments - I think we were only hoping for a quick sense check at this early stage to see if it was worth continuing. It sounds like you are receptive to the idea and there is obviously some other interest in the feature so we will proceed with docs and testings. 

We will also address your other comments.

Thanks a lot for the feedback
Chris 
haarchri: we looking for this feature to get log output from "flux check --pre"  https://github.com/fluxcd-community/helm-charts/issues/18 
Bez625: Hi @bacongobbler! We have added the testing and associated docs to the PR. Please could you review and let us know if anything needs to be changed? 🙏 
Bez625: Hi @bacongobbler is there anything else we need to do to progress this? Thanks!
bacongobbler: Hi @Bez625. Just getting back from the holidays. Apologies for the delay on a response.

I am stepping away from day-to-day code reviews for the Helm project, choosing to focus more on [reviewing new HIPs](https://github.com/helm/community/blob/main/hips/hip-0001.md), [onboarding](https://github.com/helm/helm/pull/10326) new maintainers [to the project](https://github.com/helm/community/blob/main/hips/hip-0014.md), and [reviewing incoming security audits](https://github.com/helm/community/blob/main/SECURITY.md). As a result, I will not have enough time to come back to this.

Given this PR is labeled a `size/L`, it will require approval from two core maintainers [as per the contributing policy](https://github.com/helm/helm/blob/main/CONTRIBUTING.md#size-labels). I assume everyone else was out for the holidays so I'd try asking again in a week for a review once everyone is back and all settled in.

Thanks for being patient.
Bez625: > Hi @Bez625. Just getting back from the holidays. Apologies for the delay on a response.
> 
> I am stepping away from day-to-day code reviews for the Helm project, choosing to focus more on [reviewing new HIPs](https://github.com/helm/community/blob/main/hips/hip-0001.md), [onboarding](https://github.com/helm/helm/pull/10326) new maintainers [to the project](https://github.com/helm/community/blob/main/hips/hip-0014.md), and [reviewing incoming security audits](https://github.com/helm/community/blob/main/SECURITY.md). As a result, I will not have enough time to come back to this.
> 
> Given this PR is labeled a `size/L`, it will require approval from two core maintainers [as per the contributing policy](https://github.com/helm/helm/blob/main/CONTRIBUTING.md#size-labels). I assume everyone else was out for the holidays so I'd try asking again in a week for a review once everyone is back and all settled in.
> 
> Thanks for being patient.

Thanks for replying @bacongobbler - is there anyone in particular we should be reaching out to for reviews or a github group we can tag if there is no traction within a week? 
bridgetkromhout: >  is there anyone in particular we should be reaching out to for reviews or a github group we can tag if there is no traction within a week?

Hi @Bez625, reaching out to specific maintainers probably isn't the approach that will be the most helpful. I added this to the agenda for the [upcoming weekly community call](https://github.com/helm/community/blob/main/communication.md#meetings), which you are welcome to attend.

However, most maintainers who have cycles available this week are likely working on the upcoming 3.8 release right now: https://github.com/helm/helm/milestone/113. 

If we can get your PR reviewed in the coming weeks, then it may be possible to add it to an upcoming milestone. Thanks!
mkorejo: will this enhancement allow users to view hook logs regardless of error or not?

we have a helm pre-install/pre-upgrade hook which runs DB schema migrations, and I'm wondering if this enhancement will allow me to easily view the pod logs?
Bez625: > will this enhancement allow users to view hook logs regardless of error or not?
> 
> we have a helm pre-install/pre-upgrade hook which runs DB schema migrations, and I'm wondering if this enhancement will allow me to easily view the pod logs?

Yes the output log policy is designed to be configurable. There are more details in the accompanying docs PR [here](https://github.com/helm/helm-www/pull/1242/files#diff-0415a9abbbcb28820b6339e175864f4ec055f9222050b0f5af492d611d0a2b75R217)
eabykov: @Bez625 @bacongobbler what about if the `pod` fell by mistake during helm upgrade/install?

Examples of the `helm_upgrade_logs` scripts:
- https://github.com/SamuelGarrattIqa/helm_upgrade_logs/blob/main/bin/helm_upgrade_logs.sh
- https://github.com/felipecrs/dotfiles/blob/master/home/dot_local/bin/executable_helm-upgrade-logs.tmpl

Example of GitHub actions: https://github.com/SamuelGarrattIqa/helm_upgrade_logs/actions/runs/2923030002/jobs/4661178723
Bez625: > @Bez625 @bacongobbler what about if the `pod` fell by mistake during helm upgrade/install?
> 
> Examples of the `helm_upgrade_logs` scripts:
> 
> * https://github.com/SamuelGarrattIqa/helm_upgrade_logs/blob/main/bin/helm_upgrade_logs.sh
> * https://github.com/felipecrs/dotfiles/blob/master/home/dot_local/bin/executable_helm-upgrade-logs.tmpl
> 
> Example of GitHub actions: https://github.com/SamuelGarrattIqa/helm_upgrade_logs/actions/runs/2923030002/jobs/4661178723

I'm not entirely sure I understand the question. The intention of the feature is to be able to get the output from pods or jobs created as pre or post hooks and send it to the helm client. This is configurable for either success or failure or both. 

What kind of mistake do you mean?


eabykov: > > @Bez625 @bacongobbler what about if the `pod` fell by mistake during helm upgrade/install?
> > Examples of the `helm_upgrade_logs` scripts:
> > 
> > * https://github.com/SamuelGarrattIqa/helm_upgrade_logs/blob/main/bin/helm_upgrade_logs.sh
> > * https://github.com/felipecrs/dotfiles/blob/master/home/dot_local/bin/executable_helm-upgrade-logs.tmpl
> > 
> > Example of GitHub actions: https://github.com/SamuelGarrattIqa/helm_upgrade_logs/actions/runs/2923030002/jobs/4661178723
> 
> I'm not entirely sure I understand the question. The intention of the feature is to be able to get the output from pods or jobs created as pre or post hooks and send it to the helm client. This is configurable for either success or failure or both.
> 
> What kind of mistake do you mean?

I mean the case when we don't have hooks and we just run command `helm install/upgrade`, a pod is created and we wait for it to be ready, but if it crashes with any error N times and we don't have time for the timeout, then we roll back the release. I would like to see pod shell crash logs in the console and k8s events in real time if it possible 
Bez625: > I mean the case when we don't have hooks and we just run command `helm install/upgrade`, a pod is created and we wait for it to be ready, but if it crashes with any error N times and we don't have time for the timeout, then we roll back the release. I would like to see pod shell crash logs in the console and k8s events in real time if it possible

This PR isnt intended for that particular case and is specific to hooks. However, you could potentially leverage this functionality by having a pre install hook that runs as a job checking the status/logs of your target deployment. 
Bez625: @bridgetkromhout @technosophos is there any update to this? It's been waiting over a year now!
Ornias1993: > @bridgetkromhout @technosophos is there any update to this? It's been waiting over a year now!

In general the maintainers here basically ignore feature addition PR's.
salzig: > > @bridgetkromhout @technosophos is there any update to this? It's been waiting over a year now!
> 
> In general the maintainers here basically ignore feature addition PR's.

that's very very sad. 

We would love to see this feature, so we could implement a pre-rollback that prints out some helpful hints for debugging. 
z4ce: Hey @Bez625  I brought up this PR on the developer call today. Are you still interested in this PR?

I'm looking for some guidance from @mattfarina but if things look good, would you be willing to rebase?
z4ce: I have a branch here where I've merged this patch with the current main branch of helm https://github.com/z4ce/helm/commits/output_merged
Bez625: > Hey @Bez625 I brought up this PR on the developer call today. Are you still interested in this PR?
> 
> I'm looking for some guidance from @mattfarina but if things look good, would you be willing to rebase?

Apologies I completely missed this. I'm happy to rebase if it is going to be reviewed, would you like to me to do so? 
Bez625: > I have a branch here where I've merged this patch with the current main branch of helm https://github.com/z4ce/helm/commits/output_merged

I have updated the fork and conflicts look resolved. Please let me know if there is anything else you need.
Bez625: > This looks good to me and works for some test cases I have. Approving pending HIP merge.

Thanks @joejulian! Is there anything else we need to do to get this merged?
sergelogvinov: @mattfarina @z4ce 
Hello, I saw your discussion on YouTube, and I'd like to emphasize the importance of PR.

This feature is typically uses in CI/CD pipelines like Jenkins, TeamCity and GitHub Actions. When deployments fail, developers or QA engineers often resort to checking the CI/CD logs on the CI interface. It's the most effective way to show them the reason for the deployment failure.

Flux already supports notifications, and I believe they can also integrate helm-output into the notification body.

Thank you.
gusdan: Thank for that work. We really need it!!!
LuBingtan: any updates?
mattfarina: @Bez625 Can you please rebase and fix the conflicts.
Bez625: > @Bez625 Can you please rebase and fix the conflicts.

Hi @mattfarina - I have rebased and fixed conflicts. I also replied to comment https://github.com/helm/helm/pull/10309/files#r1735244931
Bez625: Hey @mattfarina - anything else you need me to do?

Would be nice to try and get this merged, possibly even before the 3 year anniversary
Bez625: > @Bez625 thanks for all the work that went into this PR. The HIP, the functionality implementation here, and especially the tests. Also appreciate all the feedback you've incorporated into this PR, as well as the documentation PR. There is only one issue that I believe needs to be addressed.
> 
> @technosophos and @mattfarina both noted a required change that has not yet been implemented in this PR. I understand your comment here https://github.com/helm/helm/pull/10309/files#r1735244931, and I believe I understand where the confusion came from. I believe the comment you referred to is a confusing one.
> 
> The comment was added as part of #9702. I believe its intent was to explain why there was an additional interface created (to keep backwards-compatibility), and to remind us that it should be cleaned up in the next MAJOR version of Helm. However, I believe the comment is worded strangely—as if any future additions in Helm 3 should be made to this interface. That is not the case, it only explains why the previous addition was made. The fact that this comment exists does not prevent changing this interface from being a breaking change.
> 
> In short, I'd like to request you make the following changes:
> 
> 1. Create a new Interface for your addition in this PR, and update the usages accordingly. As Matt suggested, something like `InterfaceLogs` or another name you feel makes sense. Please also include a similar comment as `InterfaceExt` has (with one change, see below).
> 2. Please also clarify the wording in the comment above `InterfaceExt` to not suggest it should be used for ongoing changes. I think changing to past tense should be sufficient:
> 
> ```diff
> - // InterfaceExt is introduced to avoid breaking backwards compatibility for Interface implementers.
> + // InterfaceExt was introduced to avoid breaking backwards compatibility for Interface implementers.
> ```
> 
> 3. mark `InterfaceExt` as deprecated, and recommend using your new interface instead.

Hi @scottrigby 

Thank you for your detailed reply. I have removed the methods from the existing `InterfaceExt` and created a new `InterfaceLogs`. I have left the `InterfaceExt` definition as it was and NOT deprecated, as that seems to be the pattern with the other `Interface*` interfaces that have been introduced. I've also added `was` to my interface doc string as it seems the policy is that once an "extension" interface is merged it cannot be changed. 

Please let me know if any more changes are required.

Thanks
Bez625: > Nice work 👍
> 
> > I have left the InterfaceExt definition as it was and NOT deprecated
> 
> This is fine. My thought was that you could replace the other instances if you wanted, but that's not necessary—how you've done works just as well.
> 
> > I've also added was to my interface doc string as it seems the policy is that once an "extension" interface is merged it cannot be changed.
> 
> Just to be clear here, changing after _merge_ is not necessarily a problem, it's only a problem to change after a _release_ within the same MAJOR version, in order to fulfill Helm's strict backwards compatibility in accordance with Go's non-breaking changes.
> 
> I should note that very recently `main` is actually for Helm 4 now, but you were so close to v3 backwards compatibility I didn't want to have you reverse all that. This last change has been helpful so that a backport to `dev-v3` will be less painful (as the window for that for new features is drawing to a close). Helm 4 will be getting lots of cleanup in the near future and simplifying these interfaces can be a near-future follow up as part of that cleanup. So again, thank you.
> 
> > Please let me know if any more changes are required.
> 
> Requested changes were made, tests still look accurate and pass. I think you're good now 👍

Awesome, thanks @scottrigby . Do we need to get @mattfarina to approve or can we just dismiss his request for changes since they have been made now?
scottrigby: > Awesome, thanks @scottrigby . Do we need to get @mattfarina to approve or can we just dismiss his request for changes since they have been made now?

We need one more maintainer's approval. I raised at today's Helm dev meeting. We just have a pretty big backlog for reviews, but some folks said they'd take a look.

Matt's concern has been addressed, so we could dismiss the change request. But will keep for now in case Matt is the 2nd maintainer review in that case he can do that himself.
scottrigby: Update: https://github.com/helm/community/pull/301 has been merged as [hip-0019](https://github.com/helm/community/blob/main/hips/hip-0019.md).
Bez625: Thanks @scottrigby - please let me know if there is anything else I need to do and I'll keep an eye for second reviewer feedback. 


scottrigby: @Bez625 PR needs rebase due to go mod/sum now conflicting. This will happen periodically until this PR is merged.
Bez625: @scottrigby I have rebased

Patch:
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 8418a4c27..745f02138 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/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/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/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 b2db2ff9f..844ce161b 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


Test patch:
diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go
index 71ea83789..47cff6ec1 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_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 9f738f0bc..f78fa40d2 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_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