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

Add release tools #3

Merged
merged 181 commits into from
Oct 7, 2020

Conversation

brahmaroutu
Copy link
Contributor

Adding release tools using 'git subtree'
git subtree add --prefix=release-tools https://github.com/kubernetes-csi/csi-release-tools.git master

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 August 7, 2020 13:25
Go 1.15 was released and is the major version that Kubernetes 1.19.0
is going to use. There are probably bugs in the older 1.13.3 that were
fixed, so we should update.
Kubernetes 1.19.0 uses Go 1.15, but refers to it as 1.15.0. This broke
both the check whether we need to install 1.15 (because "go version"
reports 1.15, which didn't match 1.15.0) and then downloading the
release archive (because the URL also only uses 1.15).
It used to be necessary to override from where the E2E suite came on a
case-by-case basis (initially, testing was using a more recent suite
against an older Kubernetes). This should never become necessary again
and the lack of a specific entry for 1.18 already had the unintended
effect that Kubernetes 1.18 was tested with the suite from master, so
overall it is better to always use the E2E suite which matches
Kubernetes.
Use staging registry for canary tests
Only set staging registry when running canary job
…58f94'

git-subtree-dir: release-tools
git-subtree-mainline: 90da483
git-subtree-split: a0f195c
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 7, 2020
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 7, 2020
@brahmaroutu
Copy link
Contributor Author

#1 is closed in favor of this.

@brahmaroutu
Copy link
Contributor Author

/assign @xing-yang

@xing-yang
Copy link

It is mentioned in the following issue that we may want to move to a squash model in the future. I'd still like to see a list of individual commits if we go with the squash model.
kubernetes-csi/csi-release-tools#7 (comment)

I'm fine with the current PR. We can follow the squash model later when it is in-place.

@xing-yang
Copy link

/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 Oct 7, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brahmaroutu, xing-yang

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2020
@k8s-ci-robot k8s-ci-robot merged commit 241a7a7 into kubernetes-sigs:master Oct 7, 2020
@xing-yang
Copy link

The script imported from csi-release-tools has lots of CSI specific things including tests for CSI sidecars. Please submit followup PRs to change them accordingly.

@brahmaroutu brahmaroutu deleted the add_release_tools branch October 23, 2020 20:52
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.