Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change from disks.createSnapshot to snapshots.insert #1874

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
28 changes: 22 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 \
Expand Down
8 changes: 4 additions & 4 deletions deploy/kubernetes/images/stable-master/image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
---
78 changes: 25 additions & 53 deletions pkg/gce-cloud-provider/compute/gce-compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/gce-pd-csi-driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions some-dir
Submodule some-dir added at 78ecea
43 changes: 43 additions & 0 deletions test-config.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion test/k8s-integration/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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),
}
Expand Down
19 changes: 15 additions & 4 deletions test/run-k8s-integration-local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down