cos-tools: Regression issue for cos_gpu_installer - possible impact for user who disable network but has cached GPU drivers
Detailed description:
The root cause of this issue is that the GCS client inits at very beginning of the installer execution. This will cause the regression issue for users have cached GPU driver and use `cos-gpu-installer` with a pinned version of the installation and disabled the network.
Implementation explanation:
Lazily initialize the GCSClient within the GCSDownloader type - this is to make sure the GCSClient will only be created when necessary.
BUG=b/325342239
RELEASE_NOTE= None
TEST=test GPU driver installation with this version of gpu-installer in vm.
Change-Id: I4c95218c17602d7868ce806ceefb6b141ce6e496
Reviewed-on: https://cos-review.googlesource.com/c/cos/tools/+/65230
Tested-by: Shuo Yang <gshuoy@google.com>
Cloud-Build: GCB Service account <228075978874@cloudbuild.gserviceaccount.com>
Reviewed-by: Robert Kolchmeyer <rkolchmeyer@google.com>
Reviewed-by: Arnav Kansal <rnv@google.com>
diff --git a/src/cmd/cos_gpu_installer/internal/commands/install.go b/src/cmd/cos_gpu_installer/internal/commands/install.go
index 9578c4f..076031c 100644
--- a/src/cmd/cos_gpu_installer/internal/commands/install.go
+++ b/src/cmd/cos_gpu_installer/internal/commands/install.go
@@ -16,7 +16,6 @@
"flag"
- "cloud.google.com/go/storage"
"cos.googlesource.com/cos/tools.git/src/cmd/cos_gpu_installer/internal/installer"
"cos.googlesource.com/cos/tools.git/src/cmd/cos_gpu_installer/internal/signing"
"cos.googlesource.com/cos/tools.git/src/pkg/cos"
@@ -268,12 +267,7 @@
}
}
- gcsClient, err := storage.NewClient(ctx)
- if err != nil {
- c.logError(fmt.Errorf("failed to setup GCS client: %w", err))
- return subcommands.ExitFailure
- }
- downloader := cos.NewGCSDownloader(gcsClient, envReader, c.gcsDownloadBucket, c.gcsDownloadPrefix)
+ downloader := cos.NewGCSDownloader(nil, envReader, c.gcsDownloadBucket, c.gcsDownloadPrefix)
if c.nvidiaInstallerURL == "" {
versionInput := c.driverVersion
useFallback := useFallbackMechanism(c)
diff --git a/src/cmd/cos_gpu_installer/internal/commands/list.go b/src/cmd/cos_gpu_installer/internal/commands/list.go
index a4fbec0..e2444c8 100644
--- a/src/cmd/cos_gpu_installer/internal/commands/list.go
+++ b/src/cmd/cos_gpu_installer/internal/commands/list.go
@@ -7,7 +7,6 @@
"flag"
- "cloud.google.com/go/storage"
"cos.googlesource.com/cos/tools.git/src/cmd/cos_gpu_installer/internal/installer"
"cos.googlesource.com/cos/tools.git/src/pkg/cos"
@@ -51,13 +50,8 @@
c.logError(errors.Wrap(err, "failed to create envReader"))
return subcommands.ExitFailure
}
- gcsClient, err := storage.NewClient(ctx)
- if err != nil {
- c.logError(fmt.Errorf("failed to setup GCS client: %w", err))
- return subcommands.ExitFailure
- }
log.Infof("Running on COS build id %s", envReader.BuildNumber())
- downloader := cos.NewGCSDownloader(gcsClient, envReader, c.gcsDownloadBucket, c.gcsDownloadPrefix)
+ downloader := cos.NewGCSDownloader(nil, envReader, c.gcsDownloadBucket, c.gcsDownloadPrefix)
artifacts, err := downloader.ListGPUExtensionArtifacts()
if err != nil {
c.logError(errors.Wrap(err, "failed to list gpu extension artifacts"))
diff --git a/src/pkg/cos/artifacts.go b/src/pkg/cos/artifacts.go
index 23598d5..9e3e4e4 100644
--- a/src/pkg/cos/artifacts.go
+++ b/src/pkg/cos/artifacts.go
@@ -8,6 +8,7 @@
"path"
"path/filepath"
"strings"
+ "sync"
"cloud.google.com/go/compute/metadata"
"cloud.google.com/go/storage"
@@ -57,6 +58,7 @@
envReader *EnvReader
gcsDownloadBucket string
gcsDownloadPrefix string
+ mutex sync.Mutex
}
// NewGCSDownloader creates a GCSDownloader instance.
@@ -83,7 +85,7 @@
if prefix == "" {
prefix = path.Join(e.BuildNumber(), e.Board())
}
- return &GCSDownloader{gcsClient, e, bucket, prefix}
+ return &GCSDownloader{gcsClient: gcsClient, envReader: e, gcsDownloadBucket: bucket, gcsDownloadPrefix: prefix}
}
// DownloadKernelSrc downloads COS kernel sources to destination directory.
@@ -131,8 +133,12 @@
gcsPath := path.Join(d.gcsDownloadPrefix, artifactPath)
filename := filepath.Base(gcsPath)
outputPath := filepath.Join(destDir, filename)
+ gcsClient, err := d.getGCSClient(ctx)
+ if err != nil {
+ return err
+ }
glog.Infof("Start to download %s artifact from bucket: %s, object: %s to %s.", filename, d.gcsDownloadBucket, gcsPath, outputPath)
- if err := gcs.DownloadGCSObject(ctx, d.gcsClient, d.gcsDownloadBucket, gcsPath, outputPath); err != nil {
+ if err := gcs.DownloadGCSObject(ctx, gcsClient, d.gcsDownloadBucket, gcsPath, outputPath); err != nil {
glog.Errorf("Failed to download %s artifact from bucket: %s, object: %s to %s with error: %v.", filename, d.gcsDownloadBucket, gcsPath, outputPath, err)
return fmt.Errorf("failed to download %s artifact from bucket: %s, object: %s to %s with error: %w", filename, d.gcsDownloadBucket, gcsPath, outputPath, err)
}
@@ -145,7 +151,11 @@
func (d *GCSDownloader) ArtifactExists(ctx context.Context, artifactPath string) (bool, error) {
gcsPath := path.Join(d.gcsDownloadPrefix, artifactPath)
glog.Infof("Start to check whether artifact: %s exists in bucket: %s", gcsPath, d.gcsDownloadBucket)
- exists, error := gcs.GCSObjectExists(ctx, d.gcsClient, d.gcsDownloadBucket, gcsPath)
+ gcsClient, err := d.getGCSClient(ctx)
+ if err != nil {
+ return false, err
+ }
+ exists, error := gcs.GCSObjectExists(ctx, gcsClient, d.gcsDownloadBucket, gcsPath)
if error != nil {
glog.Errorf("Error appears when using gcs to check whether artifact: %s exists in bucket: %s - %v", gcsPath, d.gcsDownloadBucket, error)
return false, fmt.Errorf("error appears when using gcs to check whether artifact: %s exists in bucket: %s - %w", gcsPath, d.gcsDownloadBucket, error)
@@ -158,9 +168,29 @@
var objects []string
var err error
gcsPath := path.Join(d.gcsDownloadPrefix, prefix)
- if objects, err = gcs.ListGCSBucket(ctx, d.gcsClient, d.gcsDownloadBucket, gcsPath); err != nil {
+ gcsClient, err := d.getGCSClient(ctx)
+ if err != nil {
+ return nil, err
+ }
+ if objects, err = gcs.ListGCSBucket(ctx, gcsClient, d.gcsDownloadBucket, gcsPath); err != nil {
glog.Errorf("Error appears when listing artifacts in bucket: %s with prefix: %s - %v", d.gcsDownloadBucket, gcsPath, err)
return nil, fmt.Errorf("error appears when listing artifacts in bucket: %s with prefix: %s - %w", d.gcsDownloadBucket, gcsPath, err)
}
return objects, nil
}
+
+func (d *GCSDownloader) getGCSClient(ctx context.Context) (*storage.Client, error) {
+ glog.Infof("Start to fetch the GCSClient.")
+ d.mutex.Lock()
+ defer d.mutex.Unlock()
+ // Initialize the GCSClient only if it hasn't been created before.
+ if d.gcsClient == nil {
+ glog.Infof("GCSClient hasn't been created, start to create a new GCSClient.")
+ var err error
+ d.gcsClient, err = storage.NewClient(ctx)
+ if err != nil {
+ return nil, fmt.Errorf("can not create the GCSClient in GCSDownloader due to: %w", err)
+ }
+ }
+ return d.gcsClient, nil
+}
