From 271f841bc9f37371b78683e04984f39a81116601 Mon Sep 17 00:00:00 2001 From: Alexis MacAskill Date: Tue, 19 Nov 2024 03:00:02 +0000 Subject: [PATCH 1/2] change from disks.createSnapshot to snapshots.insert --- pkg/gce-cloud-provider/compute/gce-compute.go | 78 ++++++------------- pkg/gce-pd-csi-driver/controller.go | 4 + 2 files changed, 29 insertions(+), 53 deletions(-) diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index c2c2b7603..0aa9b19c0 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -1264,9 +1264,8 @@ func opIsDone(op *computev1.Operation) (bool, error) { func (cloud *CloudProvider) GetInstanceOrError(ctx context.Context, instanceZone, instanceName string) (*computev1.Instance, error) { klog.V(5).Infof("Getting instance %v from zone %v", instanceName, instanceZone) - svc := cloud.service project := cloud.project - instance, err := svc.Instances.Get(project, instanceZone, instanceName).Do() + instance, err := cloud.service.Instances.Get(project, instanceZone, instanceName).Do() if err != nil { return nil, err } @@ -1275,9 +1274,9 @@ func (cloud *CloudProvider) GetInstanceOrError(ctx context.Context, instanceZone func (cloud *CloudProvider) GetSnapshot(ctx context.Context, project, snapshotName string) (*computev1.Snapshot, error) { klog.V(5).Infof("Getting snapshot %v", snapshotName) - svc := cloud.service - snapshot, err := svc.Snapshots.Get(project, snapshotName).Context(ctx).Do() + snapshot, err := cloud.service.Snapshots.Get(project, snapshotName).Context(ctx).Do() if err != nil { + klog.V(5).Infof("Error getting snapshot %v: %v", snapshotName, err) return nil, err } return snapshot, nil @@ -1313,15 +1312,34 @@ func (cloud *CloudProvider) CreateSnapshot(ctx context.Context, project string, if description == "" { description = "Snapshot created by GCE-PD CSI Driver" } - return cloud.createZonalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams, description) case meta.Regional: if description == "" { description = "Regional Snapshot created by GCE-PD CSI Driver" } - return cloud.createRegionalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams, description) default: return nil, fmt.Errorf("could not create snapshot, key was neither zonal nor regional, instead got: %v", volKey.String()) } + + snapshotToCreate := &computev1.Snapshot{ + Name: snapshotName, + StorageLocations: snapshotParams.StorageLocations, + Description: description, + Labels: snapshotParams.Labels, + SourceDisk: cloud.GetDiskSourceURI(project, volKey), + } + _, err = cloud.service.Snapshots.Insert(project, snapshotToCreate).Context(ctx).Do() + + if err != nil { + return nil, err + } + + snapshot, err := cloud.waitForSnapshotCreation(ctx, project, snapshotName) + + if err == nil { + err = cloud.attachTagsToResource(ctx, snapshotParams.ResourceTags, project, snapshot.Id, snapshotsType, "", false, resourceManagerHostSubPath) + } + + return snapshot, err } func (cloud *CloudProvider) CreateImage(ctx context.Context, project string, volKey *meta.Key, imageName string, snapshotParams common.SnapshotParameters) (*computev1.Image, error) { @@ -1497,52 +1515,6 @@ func (cloud *CloudProvider) resizeRegionalDisk(ctx context.Context, project stri return requestGb, nil } -func (cloud *CloudProvider) createZonalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters, description string) (*computev1.Snapshot, error) { - snapshotToCreate := &computev1.Snapshot{ - Name: snapshotName, - StorageLocations: snapshotParams.StorageLocations, - Description: description, - Labels: snapshotParams.Labels, - } - - _, err := cloud.service.Disks.CreateSnapshot(project, volKey.Zone, volKey.Name, snapshotToCreate).Context(ctx).Do() - - if err != nil { - return nil, err - } - - snapshot, err := cloud.waitForSnapshotCreation(ctx, project, snapshotName) - - if err == nil { - err = cloud.attachTagsToResource(ctx, snapshotParams.ResourceTags, project, snapshot.Id, snapshotsType, "", false, resourceManagerHostSubPath) - } - - return snapshot, err -} - -func (cloud *CloudProvider) createRegionalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters, description string) (*computev1.Snapshot, error) { - snapshotToCreate := &computev1.Snapshot{ - Name: snapshotName, - StorageLocations: snapshotParams.StorageLocations, - Description: description, - Labels: snapshotParams.Labels, - } - - _, err := cloud.service.RegionDisks.CreateSnapshot(project, volKey.Region, volKey.Name, snapshotToCreate).Context(ctx).Do() - if err != nil { - return nil, err - } - - snapshot, err := cloud.waitForSnapshotCreation(ctx, project, snapshotName) - - if err == nil { - err = cloud.attachTagsToResource(ctx, snapshotParams.ResourceTags, project, snapshot.Id, snapshotsType, "", false, resourceManagerHostSubPath) - } - - return snapshot, err - -} - func (cloud *CloudProvider) waitForSnapshotCreation(ctx context.Context, project, snapshotName string) (*computev1.Snapshot, error) { ticker := time.NewTicker(time.Second) defer ticker.Stop() @@ -1558,7 +1530,7 @@ func (cloud *CloudProvider) waitForSnapshotCreation(ctx context.Context, project klog.Warningf("Error in getting snapshot %s, %v", snapshotName, err.Error()) } else if snapshot != nil { if snapshot.Status != "CREATING" { - klog.V(6).Infof("Snapshot %s status is %s", snapshotName, snapshot.Status) + klog.V(5).Infof("Snapshot %s status is %s", snapshotName, snapshot.Status) return snapshot, nil } else { klog.V(6).Infof("Snapshot %s is still creating ...", snapshotName) diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index 5a47e5da9..04babeec8 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -1670,6 +1670,8 @@ func (gceCS *GCEControllerServer) createPDSnapshot(ctx context.Context, project return nil, status.Errorf(codes.Internal, "Snapshot had error checking ready status: %v", err.Error()) } + klog.V(5).Infof("Setting csi.Snapshot %s ReadyToUse to %t based on computSnapshot.status %+v", snapshotName, ready, snapshot.Status) + return &csi.Snapshot{ SizeBytes: common.GbToBytes(snapshot.DiskSizeGb), SnapshotId: snapshotId, @@ -2078,6 +2080,8 @@ func generateDiskSnapshotEntry(snapshot *compute.Snapshot) (*csi.ListSnapshotsRe // should actually look like. ready, _ := isCSISnapshotReady(snapshot.Status) + klog.V(5).Infof("Setting csi.Snapshot %s ReadyToUse to %t based on computSnapshot.status %+v", snapshot.Name, ready, snapshot.Status) + entry := &csi.ListSnapshotsResponse_Entry{ Snapshot: &csi.Snapshot{ SizeBytes: common.GbToBytes(snapshot.DiskSizeGb), From 384ffcc8f0345268e378e937e97e1920236775c2 Mon Sep 17 00:00:00 2001 From: Alexis MacAskill Date: Thu, 21 Nov 2024 00:51:44 +0000 Subject: [PATCH 2/2] docker file --- Dockerfile | 4 +- Makefile | 28 +++++++++--- .../images/stable-master/image.yaml | 8 ++-- some-dir | 1 + test-config.yaml | 43 +++++++++++++++++++ test/k8s-integration/main.go | 3 +- test/run-k8s-integration-local.sh | 19 ++++++-- 7 files changed, 89 insertions(+), 17 deletions(-) create mode 160000 some-dir create mode 100644 test-config.yaml diff --git a/Dockerfile b/Dockerfile index aa3e287a5..bcb950dcf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,14 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM --platform=$BUILDPLATFORM golang:1.23.0 as builder +FROM --platform=linux golang:1.23.0 as builder ARG STAGINGVERSION ARG TARGETPLATFORM WORKDIR /go/src/sigs.k8s.io/gcp-compute-persistent-disk-csi-driver ADD . . -RUN GOARCH=$(echo $TARGETPLATFORM | cut -f2 -d '/') GCE_PD_CSI_STAGING_VERSION=$STAGINGVERSION make gce-pd-driver +RUN GOARCH=$(echo $TARGETPLATFORM | cut -f2 -d '/') GCE_PD_CSI_STAGING_VERSION=test-e2e-v1 make gce-pd-driver TARGETPLATFORM=linux/amd64 # Start from Kubernetes Debian base. diff --git a/Makefile b/Makefile index e77796aff..978846a38 100644 --- a/Makefile +++ b/Makefile @@ -47,11 +47,18 @@ else $(warning gcp-pd-driver-windows only supports amd64.) endif +# build-container: require-GCE_PD_CSI_STAGING_IMAGE require-GCE_PD_CSI_STAGING_VERSION init-buildx +# $(DOCKER) buildx build --platform=linux --progress=plain \ +# -t $(STAGINGIMAGE):$(STAGINGVERSION) \ +# --build-arg BUILDPLATFORM=linux \ +# --build-arg STAGINGVERSION=$(STAGINGVERSION) +# --push . + build-container: require-GCE_PD_CSI_STAGING_IMAGE require-GCE_PD_CSI_STAGING_VERSION init-buildx - $(DOCKER) buildx build --platform=linux --progress=plain \ + $(DOCKER) build --load --platform=linux --progress=plain \ -t $(STAGINGIMAGE):$(STAGINGVERSION) \ --build-arg BUILDPLATFORM=linux \ - --build-arg STAGINGVERSION=$(STAGINGVERSION) \ + --build-arg STAGINGVERSION=$(STAGINGVERSION) --push . build-and-push-windows-container-ltsc2019: require-GCE_PD_CSI_STAGING_IMAGE init-buildx @@ -88,11 +95,20 @@ validate-container-linux-arm64: init-buildx --build-arg BUILDPLATFORM=linux \ --build-arg STAGINGVERSION=$(STAGINGVERSION) . +# build-and-push-container-linux-amd64: require-GCE_PD_CSI_STAGING_IMAGE init-buildx +# $(DOCKER) buildx build --platform=linux/amd64 \ +# -t $(STAGINGIMAGE):$(STAGINGVERSION)_linux_amd64 \ +# --build-arg BUILDPLATFORM=linux \ +# --build-arg STAGINGVERSION=$(STAGINGVERSION) --push . + + build-and-push-container-linux-amd64: require-GCE_PD_CSI_STAGING_IMAGE init-buildx - $(DOCKER) buildx build --platform=linux/amd64 \ - -t $(STAGINGIMAGE):$(STAGINGVERSION)_linux_amd64 \ - --build-arg BUILDPLATFORM=linux \ - --build-arg STAGINGVERSION=$(STAGINGVERSION) --push . + $(DOCKER) build -f . + + # --platform=linux/amd64 \ + # -t $(STAGINGIMAGE):$(STAGINGVERSION)_linux_amd64 \ + # --build-arg BUILDPLATFORM=linux \ + # --build-arg STAGINGVERSION=$(STAGINGVERSION) --push . build-and-push-container-linux-arm64: require-GCE_PD_CSI_STAGING_IMAGE init-buildx $(DOCKER) buildx build --file=Dockerfile --platform=linux/arm64 \ diff --git a/deploy/kubernetes/images/stable-master/image.yaml b/deploy/kubernetes/images/stable-master/image.yaml index 2d2df2367..270a7bf4b 100644 --- a/deploy/kubernetes/images/stable-master/image.yaml +++ b/deploy/kubernetes/images/stable-master/image.yaml @@ -30,8 +30,8 @@ kind: ImageTagTransformer metadata: name: imagetag-csi-snapshotter imageTag: - name: registry.k8s.io/sig-storage/csi-snapshotter - newTag: "v6.3.3" + name: us-central1-docker.pkg.dev/amacaskill-gke-dev/external-snapshotter/logging-image:latest + newTag: "latest" --- apiVersion: builtin @@ -51,6 +51,6 @@ imageTag: name: gke.gcr.io/gcp-compute-persistent-disk-csi-driver # Don't change stable image without changing pdImagePlaceholder in # test/k8s-integration/main.go - newName: registry.k8s.io/cloud-provider-gcp/gcp-compute-persistent-disk-csi-driver - newTag: "v1.15.0" + newName: us-central1-docker.pkg.dev/amacaskill-gke-dev/gcp-compute-persistent-disk-csi-driver/logging-image:latest + newTag: "latest" --- diff --git a/some-dir b/some-dir new file mode 160000 index 000000000..78ecea5a7 --- /dev/null +++ b/some-dir @@ -0,0 +1 @@ +Subproject commit 78ecea5a708046ee2d4e71be5dc73393b8d7d7cc diff --git a/test-config.yaml b/test-config.yaml new file mode 100644 index 000000000..95ba795ef --- /dev/null +++ b/test-config.yaml @@ -0,0 +1,43 @@ +StorageClass: + FromFile: /usr/local/google/home/amacaskill/go/src/sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/k8s-integration/config/sc-standard.yaml + +SnapshotClass: + FromFile: /usr/local/google/home/amacaskill/go/src/sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/k8s-integration/config/pd-volumesnapshotclass.yaml + + +Timeouts: + DataSourceProvision: 480s + + +DriverInfo: + Name: csi-gcepd-sc-standard--pd-volumesnapshotclass + SupportedFsType: + ext2: + ext3: + ext4: + xfs: + + Capabilities: + persistence: true + block: true + fsGroup: true + exec: true + multipods: true + topology: true + controllerExpansion: true + nodeExpansion: true + snapshotDataSource: true + pvcDataSource: true + + StressTestOptions: + NumPods: 10 + NumRestarts: 10 + SupportedMountOption: + debug: + noatime: + SupportedSizeRange: + Min: 5Gi + Max: 64Ti + TopologyKeys: + - topology.gke.io/zone + NumAllowedTopologies: 1 diff --git a/test/k8s-integration/main.go b/test/k8s-integration/main.go index 092526a8b..f10d4ec5d 100644 --- a/test/k8s-integration/main.go +++ b/test/k8s-integration/main.go @@ -133,7 +133,7 @@ func main() { } if *useGKEManagedDriver { - ensureVariableVal(deploymentStrat, "gke", "deployment strategy must be GKE for using managed driver") + // ensureVariableVal(deploymentStrat, "gke", "deployment strategy must be GKE for using managed driver") ensureFlag(doDriverBuild, false, "'do-driver-build' must be false when using GKE managed driver") ensureFlag(teardownDriver, false, "'teardown-driver' must be false when using GKE managed driver") ensureVariable(stagingImage, false, "'staging-image' must not be set when using GKE managed driver") @@ -904,6 +904,7 @@ func runTestsWithConfig(testParams *testParameters, testConfigArg, reportPrefix kubeTestArgs := []string{ "--test", "--ginkgo-parallel", + "--provider=gce", "--check-version-skew=false", fmt.Sprintf("--test_args=%s", testArgs), } diff --git a/test/run-k8s-integration-local.sh b/test/run-k8s-integration-local.sh index c551b0212..695530894 100755 --- a/test/run-k8s-integration-local.sh +++ b/test/run-k8s-integration-local.sh @@ -32,6 +32,17 @@ make -C "${PKGDIR}" test-k8s-integration # --deployment-strategy=gce --kube-version=${kube_version} \ # --test-version=${test_version} --num-nodes=3 +${PKGDIR}/bin/k8s-integration-test --run-in-prow=false \ + --test-focus="External.Storage.*VolumeSnapshotDataSource" \ + --local-k8s-dir=$KTOP --storageclass-files=sc-standard.yaml \ + --snapshotclass-files=pd-volumesnapshotclass.yaml \ + --do-driver-build=false --teardown-driver=false \ + --gce-zone="us-central1-c" --num-nodes=${NUM_NODES:-3} \ + --use-gke-managed-driver=true \ + --deployment-strategy="gke" \ + --teardown-cluster=false --bringup-cluster=false \ + --gke-cluster-version="1.31" --gke-cluster-name="amacaskill-cluster" + # This version of the command creates a regional GKE cluster. It will test with # the latest GKE version and the master test version @@ -94,10 +105,10 @@ make -C "${PKGDIR}" test-k8s-integration # # As with all other methods local credentials must be set by running # gcloud auth application-default login -"${PKGDIR}/bin/k8s-integration-test" --run-in-prow=false \ ---deploy-overlay-name=noauth --bringup-cluster=false --teardown-cluster=false --local-k8s-dir="$KTOP" \ ---storageclass-files=sc-standard.yaml --do-driver-build=false --test-focus='External.Storage' \ ---gce-zone="us-central1-b" --num-nodes="${NUM_NODES:-3}" +# "${PKGDIR}/bin/k8s-integration-test" --run-in-prow=false \ +# --deploy-overlay-name=noauth --bringup-cluster=false --teardown-cluster=false --local-k8s-dir="$KTOP" \ +# --storageclass-files=sc-standard.yaml --do-driver-build=false --test-focus='External.Storage' \ +# --gce-zone="us-central1-b" --num-nodes="${NUM_NODES:-3}" # This version of the command does not build the driver or K8s, points to a