search

kubernetes-sigs / aws-ebs-csi-driver

CSI driver for Amazon EBS https://aws.amazon.com/ebs/
Apache License 2.0
974 stars 787 forks source link

Developer Experience Improvements #2079

Closed ConnorJC3 closed 2 months ago

ConnorJC3 commented 3 months ago

Is this a bug fix or adding new feature?

Neither - it focuses on improvements to the developer experience.

What is this PR about? / Why do we need it?

This PR contains several improvements split up by commit:

Only collect metrics in Prow

Metrics collection runs locally by default, but this makes no sense because the metrics will fail to publish anyways. Check for the presence of PROW_JOB_ID and only run metrics in Prow jobs.

Simplify image building

Simplifies handling of image building in the Makefile. Removes some CI-only defaults from the Makefile that are confusing and difficult to use. Renames hack/prow.sh to hack/cloudbuild.sh to clearly indicate what it is used for.

Split driver install/uninstall out to functions; Introduce make cluster/(un)install

Introduces make cluster/install and make cluster/uninstall which can be used to (un)install the driver for local testing. In order to accomplish this, driver installation and uninstallation are split out into a separate function in utils.sh

Cache GOCACHE and GOMODCACHE in Dockerfile

Configures the Dockerfile to cache go modules and build output on supported platforms. In my local testing, this introduced an approximate 30x speedup (from 600 seconds to 20 seconds) when rebuilding the driver image with a small change (such as a dependency upgrade) to go.mod.

Update Makefile documentation and CONTRIBUTING.md

Makefile documentation was somewhat out of date, cleans it up. Also moves the quickstart guide to CONTRIBUTING.md and cleans up outdated information in CONTRIBUTING.md.

Cleanup non-external e2e test handling

Cleans up the logic of non-external tests, fixing a bug that causes the tests to fail to run when run from inside a home directory that is a symlink.

What testing is done?

CI/Manual

github-actions[bot] commented 3 months ago

Code Coverage Diff

This PR does not change the code coverage

AndrewSirenko commented 3 months ago

Thanks for these amazing devloop + new contributor improvements.

TIL: cache mounts... (Thank you for this...)

xref: #1929 (Because this PR resolves checklist items 1,2, and 3)

Working my way through testing the PR, eta end of week.

AndrewSirenko commented 2 months ago

/lgtm

Will approve once devloop with EKS confirmed works.

AndrewSirenko commented 2 months ago

/lgtm /approve

ConnorJC3 commented 2 months ago

/retest

k8s-ci-robot commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndrewSirenko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/OWNERS)~~ [AndrewSirenko] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
AndrewSirenko commented 2 months ago

/lgtm