msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

Merge pull request #31717 from AustinAbro321/use-logger-with-waiter

Browse Source 
fix: use kube logger with status waiter
main
Scott Rigby 1 day ago committed by GitHub
parent 9211dfef6a ec0726523e
commit f928025cdb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 55 additions and 42 deletions
Show Stats Download Patch File Download Diff File

6
pkg/kube/client.go
Unescape View File

@ -166,7 +166,7 @@ func (c *Client) newStatusWatcher(opts ...WaitOption) (*statusWaiter, error) {
if waitContext == nil {
waitContext = c.WaitContext
}
return &statusWaiter{
sw := &statusWaiter{
restMapper: restMapper,
client: dynamicClient,
ctx: waitContext,
@ -175,7 +175,9 @@ func (c *Client) newStatusWatcher(opts ...WaitOption) (*statusWaiter, error) {
waitWithJobsCtx: o.waitWithJobsCtx,
waitForDeleteCtx: o.waitForDeleteCtx,
readers: o.statusReaders,
}, nil
}
sw.SetLogger(c.Logger().Handler())
return sw, nil
}
func (c *Client) GetWaiter(ws WaitStrategy) (Waiter, error) {

20
pkg/kube/statuswait.go
Unescape View File

@ -38,6 +38,7 @@ import (
"k8s.io/client-go/dynamic"
watchtools "k8s.io/client-go/tools/watch"
"helm.sh/helm/v4/internal/logging"
helmStatusReaders "helm.sh/helm/v4/internal/statusreaders"
)
@ -50,6 +51,7 @@ type statusWaiter struct {
waitWithJobsCtx context.Context
waitForDeleteCtx context.Context
readers []engine.StatusReader
logging.LogHolder
}
// DefaultStatusWatcherTimeout is the timeout used by the status waiter when a
@ -72,7 +74,7 @@ func (w *statusWaiter) WatchUntilReady(resourceList ResourceList, timeout time.D
}
ctx, cancel := w.contextWithTimeout(w.watchUntilReadyCtx, timeout)
defer cancel()
slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
w.Logger().Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)
jobSR := helmStatusReaders.NewCustomJobStatusReader(w.restMapper)
podSR := helmStatusReaders.NewCustomPodStatusReader(w.restMapper)
@ -94,7 +96,7 @@ func (w *statusWaiter) Wait(resourceList ResourceList, timeout time.Duration) er
}
ctx, cancel := w.contextWithTimeout(w.waitCtx, timeout)
defer cancel()
slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
w.Logger().Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)
sw.StatusReader = statusreaders.NewStatusReader(w.restMapper, w.readers...)
return w.wait(ctx, resourceList, sw)
@ -106,7 +108,7 @@ func (w *statusWaiter) WaitWithJobs(resourceList ResourceList, timeout time.Dura
}
ctx, cancel := w.contextWithTimeout(w.waitWithJobsCtx, timeout)
defer cancel()
slog.Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
w.Logger().Debug("waiting for resources", "count", len(resourceList), "timeout", timeout)
sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)
newCustomJobStatusReader := helmStatusReaders.NewCustomJobStatusReader(w.restMapper)
readers := append([]engine.StatusReader(nil), w.readers...)
@ -122,7 +124,7 @@ func (w *statusWaiter) WaitForDelete(resourceList ResourceList, timeout time.Dur
}
ctx, cancel := w.contextWithTimeout(w.waitForDeleteCtx, timeout)
defer cancel()
slog.Debug("waiting for resources to be deleted", "count", len(resourceList), "timeout", timeout)
w.Logger().Debug("waiting for resources to be deleted", "count", len(resourceList), "timeout", timeout)
sw := watcher.NewDefaultStatusWatcher(w.client, w.restMapper)
return w.waitForDelete(ctx, resourceList, sw)
}
@ -142,7 +144,7 @@ func (w *statusWaiter) waitForDelete(ctx context.Context, resourceList ResourceL
RESTScopeStrategy: watcher.RESTScopeNamespace,
})
statusCollector := collector.NewResourceStatusCollector(resources)
done := statusCollector.ListenWithObserver(eventCh, statusObserver(cancel, status.NotFoundStatus))
done := statusCollector.ListenWithObserver(eventCh, statusObserver(cancel, status.NotFoundStatus, w.Logger()))
<-done
if statusCollector.Error != nil {
@ -189,7 +191,7 @@ func (w *statusWaiter) wait(ctx context.Context, resourceList ResourceList, sw w
RESTScopeStrategy: watcher.RESTScopeNamespace,
})
statusCollector := collector.NewResourceStatusCollector(resources)
done := statusCollector.ListenWithObserver(eventCh, statusObserver(cancel, status.CurrentStatus))
done := statusCollector.ListenWithObserver(eventCh, statusObserver(cancel, status.CurrentStatus, w.Logger()))
<-done
if statusCollector.Error != nil {
@ -228,7 +230,7 @@ func contextWithTimeout(ctx context.Context, timeout time.Duration) (context.Con
return watchtools.ContextWithOptionalTimeout(ctx, timeout)
}
func statusObserver(cancel context.CancelFunc, desired status.Status) collector.ObserverFunc {
func statusObserver(cancel context.CancelFunc, desired status.Status, logger *slog.Logger) collector.ObserverFunc {
return func(statusCollector *collector.ResourceStatusCollector, _ event.Event) {
var rss []*event.ResourceStatus
var nonDesiredResources []*event.ResourceStatus
@ -253,7 +255,7 @@ func statusObserver(cancel context.CancelFunc, desired status.Status) collector.
}
if aggregator.AggregateStatus(rss, desired) == desired {
slog.Debug("all resources achieved desired status", "desiredStatus", desired, "resourceCount", len(rss))
logger.Debug("all resources achieved desired status", "desiredStatus", desired, "resourceCount", len(rss))
cancel()
return
}
@ -264,7 +266,7 @@ func statusObserver(cancel context.CancelFunc, desired status.Status) collector.
return nonDesiredResources[i].Identifier.Name < nonDesiredResources[j].Identifier.Name
})
first := nonDesiredResources[0]
slog.Debug("waiting for resource", "namespace", first.Identifier.Namespace, "name", first.Identifier.Name, "kind", first.Identifier.GroupKind.Kind, "expectedStatus", desired, "actualStatus", first.Status)
logger.Debug("waiting for resource", "namespace", first.Identifier.Namespace, "name", first.Identifier.Name, "kind", first.Identifier.GroupKind.Kind, "expectedStatus", desired, "actualStatus", first.Status)
}
}
}

71
pkg/kube/statuswait_test.go
Unescape View File

@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"
"strings"
"sync/atomic"
"testing"
@ -326,6 +327,7 @@ func TestStatusWaitForDelete(t *testing.T) {
restMapper: fakeMapper,
client: fakeClient,
}
statusWaiter.SetLogger(slog.Default().Handler())
objsToCreate := getRuntimeObjFromManifests(t, tt.manifestsToCreate)
for _, objToCreate := range objsToCreate {
u := objToCreate.(*unstructured.Unstructured)
@ -369,6 +371,7 @@ func TestStatusWaitForDeleteNonExistentObject(t *testing.T) {
restMapper: fakeMapper,
client: fakeClient,
}
statusWaiter.SetLogger(slog.Default().Handler())
// Don't create the object to test that the wait for delete works when the object doesn't exist
objManifest := getRuntimeObjFromManifests(t, []string{podCurrentManifest})
resourceList := getResourceListFromRuntimeObjs(t, c, objManifest)
@ -425,6 +428,7 @@ func TestStatusWait(t *testing.T) {
client: fakeClient,
restMapper: fakeMapper,
}
statusWaiter.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -481,6 +485,7 @@ func TestWaitForJobComplete(t *testing.T) {
client: fakeClient,
restMapper: fakeMapper,
}
statusWaiter.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -543,6 +548,7 @@ func TestWatchForReady(t *testing.T) {
client: fakeClient,
restMapper: fakeMapper,
}
statusWaiter.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -570,19 +576,19 @@ func TestStatusWaitMultipleNamespaces(t *testing.T) {
name string
objManifests []string
expectErrStrs []string
testFunc func(statusWaiter, ResourceList, time.Duration) error
testFunc func(*statusWaiter, ResourceList, time.Duration) error
}{
{
name: "pods in multiple namespaces",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
{
name: "hooks in multiple namespaces",
objManifests: []string{jobNamespace1CompleteManifest, podNamespace2SucceededManifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WatchUntilReady(rl, timeout)
},
},
@ -590,42 +596,42 @@ func TestStatusWaitMultipleNamespaces(t *testing.T) {
name: "error when resource not ready in one namespace",
objManifests: []string{podNamespace1NoStatusManifest, podNamespace2Manifest},
expectErrStrs: []string{"resource Pod/namespace-1/pod-ns1 not ready. status: InProgress", "context deadline exceeded"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
{
name: "delete resources in multiple namespaces",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitForDelete(rl, timeout)
},
},
{
name: "cluster-scoped resources work correctly with unrestricted permissions",
objManifests: []string{podNamespace1Manifest, clusterRoleManifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
{
name: "namespace-scoped and cluster-scoped resources work together",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest, clusterRoleManifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
{
name: "delete cluster-scoped resources works correctly",
objManifests: []string{podNamespace1Manifest, namespaceManifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitForDelete(rl, timeout)
},
},
{
name: "watch cluster-scoped resources works correctly",
objManifests: []string{clusterRoleManifest},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WatchUntilReady(rl, timeout)
},
},
@ -646,6 +652,7 @@ func TestStatusWaitMultipleNamespaces(t *testing.T) {
client: fakeClient,
restMapper: fakeMapper,
}
sw.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -668,7 +675,7 @@ func TestStatusWaitMultipleNamespaces(t *testing.T) {
}
resourceList := getResourceListFromRuntimeObjs(t, c, objs)
err := tt.testFunc(sw, resourceList, time.Second*3)
err := tt.testFunc(&sw, resourceList, time.Second*3)
if tt.expectErrStrs != nil {
require.Error(t, err)
for _, expectedErrStr := range tt.expectErrStrs {
@ -756,13 +763,13 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
objManifests []string
allowedNamespaces []string
expectErrs []error
testFunc func(statusWaiter, ResourceList, time.Duration) error
testFunc func(*statusWaiter, ResourceList, time.Duration) error
}{
{
name: "pods in multiple namespaces with namespace permissions",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
allowedNamespaces: []string{"namespace-1", "namespace-2"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -770,7 +777,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
name: "delete pods in multiple namespaces with namespace permissions",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
allowedNamespaces: []string{"namespace-1", "namespace-2"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitForDelete(rl, timeout)
},
},
@ -778,7 +785,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
name: "hooks in multiple namespaces with namespace permissions",
objManifests: []string{jobNamespace1CompleteManifest, podNamespace2SucceededManifest},
allowedNamespaces: []string{"namespace-1", "namespace-2"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WatchUntilReady(rl, timeout)
},
},
@ -787,7 +794,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
objManifests: []string{podNamespace1Manifest, clusterRoleManifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have cluster-wide LIST permissions for cluster-scoped resources")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -796,7 +803,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
objManifests: []string{podNamespace1Manifest, namespaceManifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have cluster-wide LIST permissions for cluster-scoped resources")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitForDelete(rl, timeout)
},
},
@ -805,7 +812,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have LIST permissions in namespace %q", "namespace-2")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -827,6 +834,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
client: baseFakeClient,
restMapper: fakeMapper,
}
sw.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -849,7 +857,7 @@ func TestStatusWaitRestrictedRBAC(t *testing.T) {
}
resourceList := getResourceListFromRuntimeObjs(t, c, objs)
err := tt.testFunc(sw, resourceList, time.Second*3)
err := tt.testFunc(&sw, resourceList, time.Second*3)
if tt.expectErrs != nil {
require.Error(t, err)
for _, expectedErr := range tt.expectErrs {
@ -870,13 +878,13 @@ func TestStatusWaitMixedResources(t *testing.T) {
objManifests []string
allowedNamespaces []string
expectErrs []error
testFunc func(statusWaiter, ResourceList, time.Duration) error
testFunc func(*statusWaiter, ResourceList, time.Duration) error
}{
{
name: "wait succeeds with namespace-scoped resources only",
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
allowedNamespaces: []string{"namespace-1", "namespace-2"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -885,7 +893,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
objManifests: []string{podNamespace1Manifest, clusterRoleManifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have cluster-wide LIST permissions for cluster-scoped resources")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -894,7 +902,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
objManifests: []string{podNamespace1Manifest, clusterRoleManifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have cluster-wide LIST permissions for cluster-scoped resources")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitForDelete(rl, timeout)
},
},
@ -903,7 +911,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
objManifests: []string{podNamespace1Manifest, namespaceManifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have cluster-wide LIST permissions for cluster-scoped resources")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -912,7 +920,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
objManifests: []string{podNamespace1Manifest, podNamespace2Manifest},
allowedNamespaces: []string{"namespace-1"},
expectErrs: []error{fmt.Errorf("user does not have LIST permissions in namespace %q", "namespace-2")},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -934,6 +942,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
client: baseFakeClient,
restMapper: fakeMapper,
}
sw.SetLogger(slog.Default().Handler())
objs := getRuntimeObjFromManifests(t, tt.objManifests)
for _, obj := range objs {
u := obj.(*unstructured.Unstructured)
@ -956,7 +965,7 @@ func TestStatusWaitMixedResources(t *testing.T) {
}
resourceList := getResourceListFromRuntimeObjs(t, c, objs)
err := tt.testFunc(sw, resourceList, time.Second*3)
err := tt.testFunc(&sw, resourceList, time.Second*3)
if tt.expectErrs != nil {
require.Error(t, err)
for _, expectedErr := range tt.expectErrs {
@ -1151,7 +1160,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
objManifests []string
customReader *mockStatusReader
expectErrStrs []string
testFunc func(statusWaiter, ResourceList, time.Duration) error
testFunc func(*statusWaiter, ResourceList, time.Duration) error
}{
{
name: "Wait returns error when resource has failed",
@ -1161,7 +1170,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
status: status.FailedStatus,
},
expectErrStrs: []string{"resource Pod/ns/in-progress-pod not ready. status: Failed, message: mock status reader"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -1172,7 +1181,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
expectErrStrs: []string{
"resource Job/default/failed-job not ready. status: Failed",
},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WaitWithJobs(rl, timeout)
},
},
@ -1188,7 +1197,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
"resource Pod/ns/in-progress-pod not ready. status: Failed, message: mock status reader",
"resource Pod/ns/current-pod not ready. status: Failed, message: mock status reader",
},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.Wait(rl, timeout)
},
},
@ -1201,7 +1210,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
},
// WatchUntilReady also waits for CurrentStatus, so failed resources should return error
expectErrStrs: []string{"resource Pod/ns/in-progress-pod not ready. status: Failed, message: mock status reader"},
testFunc: func(sw statusWaiter, rl ResourceList, timeout time.Duration) error {
testFunc: func(sw *statusWaiter, rl ResourceList, timeout time.Duration) error {
return sw.WatchUntilReady(rl, timeout)
},
},
@ -1233,7 +1242,7 @@ func TestStatusWaitWithFailedResources(t *testing.T) {
assert.NoError(t, err)
}
resourceList := getResourceListFromRuntimeObjs(t, c, objs)
err := tt.testFunc(sw, resourceList, time.Second*3)
err := tt.testFunc(&sw, resourceList, time.Second*3)
if tt.expectErrStrs != nil {
require.Error(t, err)
for _, expectedErrStr := range tt.expectErrStrs {

Write Preview
Loading…
Cancel
Save