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

Setup automated builds using the release-tools #30

Merged
merged 203 commits into from
Dec 15, 2020

Conversation

kmova
Copy link
Contributor

@kmova kmova commented Dec 14, 2020

Followed the steps listed here (https://github.com/kubernetes-csi/csi-release-tools)
to setup automated builds via release-tools.

  • add release-tools via git subtree
  • update Makefile for arm builds to use the release-tools/make
  • add cloudbuild and travis links from release-tools
  • update Dockerfile for amd64 as expected by release tools

All the commits except (1311c59) are from git add subtree.

Verified that following commands are working:

make all
make
make container
make container-arm

spiffxp and others added 30 commits January 16, 2019 10:47
The repo was created with an HTML version of the build.make file from
https://github.com/pohly/csi-build-rules/. Here's the raw file.
It's worth calling out explicitly that only the master branch is
maintained.
The actual repository was not named like the prototype repo.
Copy-and-paste error from the time when the
kubernetes-csi/csi-release-tools repo didn't have the code...
README.md: fix repo URL for initial setup
The goal is to enforce that changes get merged upstream first and only
get into the local repo via a normal "git subtree merge".
"make test" used to abort after the first test failure. That was
partly intentional: if the simple tests already fail (for example,
because of a syntax error), then there is no point in continuing to
test.

However, it also makes it harder to find all errors in a CI system
when the errors are unrelated (first error shows up, gets fixed, next
error shows up, etc.).

Now "make test" still aborts early, but "make -k test" is used in the
CI and will run all individual tests because they are split up into
different targets.
We don't want to allow local modifications in the subtree. Everything
should go to the csi-release-tools repo first.
This may or may not work, depending on which packages have tests and
whether they contain glog.
Individual repos may have to filter out certain packages from
testing. For example, in csi-test the cmd/csi-sanity directory
contains a special test that depends on additional parameters that set
the CSI driver to test against.
After merging into external-attacher, the next Travis CI run did not
push the "canary" image because the check for "canary" only covered
the case where "-canary" is used as
suffix (https://travis-ci.org/kubernetes-csi/external-attacher/builds/484095261).
The introduction for each individual test looked like an actual
command:

  test-subtree
  ./release-tools/verify-subtree.sh release-tools
  Directory 'release-tools' contains non-upstream changes:
  ...

It's better to make it look like a shell comment and increase its
visibility with a longer prefix:

  ### test-subtree:
  ./release-tools/verify-subtree.sh release-tools
  ...
build.make: fix pushing of "canary" image from master branch
If for whatever reasons a repo already had a `release-tools` directory
before doing a clean import of it with `git subtree`, the check used
to fail because it found those old commits.

This can be fixed by telling `git log` to stop when the directory
disappears from the repo. There has to be a commit with removes the
old content, because otherwise `git subtree add` doesn't work.

Fixes: kubernetes-csi/external-resizer#21
verify-subtree.sh: relax check and ignore old content
In repos that have a test/e2e, that test suite should be run
separately because it depends on a running cluster.
This is an unmodified copy of kubernetes/hack/verify-shellcheck.sh
revision d5a3db003916b1d33b503ccd2e4897e094d8af77.
This runs "dep check" to verify that the vendor directory is
up-to-date and meets expectations (= done with dep >= 0.5.0).
In repos that have a test/e2e, that test suite should be run
separately because it depends on a running cluster.
build.make: avoid unit-testing E2E test suite
These are the modifications that were necessary to call this outside
of Kubernetes. The support for excluding files from checking gets
removed to simplify the script. It shouldn't be needed, because
linting can be enabled after fixing whatever scripts might fail the
check.
By default this only tests the scripts inside the "release-tools"
directory, which is useful when making experimental changes to them in
a component that uses csi-release-tools. But a component can also
enable checking for other directories.
This enables testing of other repos and of this repo itself inside
Prow. Currently supported is unit testing ("make test") and E2E
testing (either via a local test suite or the Kubernetes E2E test
suite applied to the hostpath driver example deployment).

The script passes shellcheck and uses Prow to verify that for future
PRs.
k8s-ci-robot and others added 14 commits October 27, 2020 09:54
…d-workaround

prow.sh: work around "kind build node-image" failure
prow.sh: only run alpha tests for latest Kubernetes release
This PR installs snapshot CRDs and rbac rules from the repo
and installs snapshot controller from a local image if it is
a PR in external-snapshotter repo.
Otherwise it uses main or a stable version.
…t_controller

Fix the install of snapshot CRDs and controller
Builds of external-snapshotter sometimes timed out because it builds
three images and that takes almost half an hour (the previous limit)
and more if the network is slow.

There's no good way to set timeouts per repo and a too high timeout
for shouldn't cause any issue, therefore the timeout gets increased
everywhere.
cloud build: double the timeout, now 1 hour
…82af4'

git-subtree-dir: release-tools
git-subtree-mainline: 1662cc7
git-subtree-split: 3b6d17b
Follow the steps here to setup automated builds via
release-tools.

https://github.com/kubernetes-csi/csi-release-tools

- add release-tools via git subtree
- update Makefile for arm builds to use the release-tools/make
- add cloudbuild and travis links from release-tools
- update Dockerfile for amd64 as expected by release tools

Signed-off-by: kmova <[email protected]>
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 14, 2020
@kmova
Copy link
Contributor Author

kmova commented Dec 14, 2020

@msau42 @pohly - can you please help with reviews on this PR.

build-docker build-docker-x86_64:
GOOS=linux go build -o deploy/docker/x86_64/nfs-provisioner ./cmd/nfs-provisioner
.PHONY: build-docker build-docker-x86_64
IMAGE_ARM = $(REGISTRY)nfs-provisioner-arm:$(VERSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cloud build and release tools already supports multiarch builds. I'm not sure we need these arm specific params and commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dockerfiles for arm and amd64 are a bit different at the moment. We need to work on refactoring them to multi-arch compatible Docker files. Prior to refactoring, we need to add fix the e2e tests.

The current users are building with the existing Docker files and make commands in forked repos - so tried to maintain that backward compatibility for now till the refactoring is completed.

go test `go list ./... | grep -v 'vendor\|test\|demo'`
.PHONY: test

test-e2e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a replacement for test-e2e?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, this is coming up in a follow-up PR. Currently, these tests are out of date and not being used.

@@ -0,0 +1 @@
./release-tools/travis.yml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably don't need travis if we enable all testing via prow

@msau42
Copy link
Contributor

msau42 commented Dec 15, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 15, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kmova, msau42

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.