From daf4ef232890201705c9831bd3230b580ff00239 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 9 Oct 2020 10:39:06 -0400 Subject: [PATCH 1/3] csi: fix incorrect comment on csi_hook context lifetime --- client/allocrunner/csi_hook.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/client/allocrunner/csi_hook.go b/client/allocrunner/csi_hook.go index 1b03d301f19..3f667363539 100644 --- a/client/allocrunner/csi_hook.go +++ b/client/allocrunner/csi_hook.go @@ -34,12 +34,10 @@ func (c *csiHook) Prerun() error { return nil } - // TODO(tgross): the contexts for the CSI RPC calls made during - // mounting can have very long timeouts. Until these are better - // tuned, there's not a good value to put here for a WithCancel - // without risking conflicts with the grpc retries/timeouts in the - // pluginmanager package. - ctx := context.TODO() + // We use this context only to attach hclog to the gRPC context. The + // lifetime is the lifetime of the gRPC stream, not specific RPC timeouts, + // but we manage the stream lifetime via Close in the pluginmanager. + ctx := context.Background() volumes, err := c.claimVolumesFromAlloc() if err != nil { From c0b5cbebd3e2f518796c5d185a64f64e1d6aa479 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 9 Oct 2020 10:39:25 -0400 Subject: [PATCH 2/3] csi: remove stray TODO comment This item was completed in #8626 --- nomad/csi_endpoint.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index ed95e994f30..78655f4a8df 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -521,8 +521,6 @@ func (v *CSIVolume) Unpublish(args *structs.CSIVolumeUnpublishRequest, reply *st metricsStart := time.Now() defer metrics.MeasureSince([]string{"nomad", "volume", "unpublish"}, metricsStart) - // TODO(tgross): ensure we have pass-thru of token for client-driven RPC - // ref https://github.com/hashicorp/nomad/issues/8373 allowVolume := acl.NamespaceValidator(acl.NamespaceCapabilityCSIMountVolume) aclObj, err := v.srv.WriteACLObj(&args.WriteRequest, true) if err != nil { From 323cb797419e05cbf70f467cffc31e78b7e42b93 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 9 Oct 2020 10:39:54 -0400 Subject: [PATCH 3/3] csi: volumewatcher only needs one pass to collect past claims If a volume GC and a `nomad volume detach` command land concurrently, we can end up with multiple claims without an allocation, which results in extra no-op work when finding claims to collect as past claims. --- nomad/volumewatcher/volume_watcher.go | 1 + 1 file changed, 1 insertion(+) diff --git a/nomad/volumewatcher/volume_watcher.go b/nomad/volumewatcher/volume_watcher.go index 9137d721e2b..80304c57440 100644 --- a/nomad/volumewatcher/volume_watcher.go +++ b/nomad/volumewatcher/volume_watcher.go @@ -177,6 +177,7 @@ func (vw *volumeWatcher) volumeReapImpl(vol *structs.CSIVolume) error { for _, claim := range vol.PastClaims { if claim.AllocationID == "" { vol = vw.collectPastClaims(vol) + break // only need to collect once } }