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

release tools #6

Merged
merged 21 commits into from
Feb 20, 2019
Merged

release tools #6

merged 21 commits into from
Feb 20, 2019

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Feb 20, 2019

This imports the shared csi-release-tools and uses it for Travis CI
and building. The binary and image name are expected to be "iscsiplugin",
but a Dockerfile still needs to be added by someone who knows what needs
to be in it.

/assign @msau42

spiffxp and others added 21 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
…631f2'

git-subtree-dir: release-tools
git-subtree-mainline: 627fdbf
git-subtree-split: 33d58fd
This imports the shared csi-release-tools and uses it for Travis CI
and building. Everything is set up to build the "iscsiplugin" binary
and image, but as there is no Dockerfile yet, image creation and
pushing is not active.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 20, 2019
@pohly
Copy link
Contributor Author

pohly commented Feb 20, 2019

Travis CI still needs to be activated for this repo. This can only be done by someone in the kubernetes-csi admin group.

@msau42
Copy link
Collaborator

msau42 commented Feb 20, 2019

/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 Feb 20, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Feb 20, 2019
@k8s-ci-robot k8s-ci-robot merged commit 93379ae into kubernetes-csi:master Feb 20, 2019
@msau42
Copy link
Collaborator

msau42 commented Feb 22, 2019

I enabled travis. We'll see on the next pull if it works. I tried to manually trigger a build, but it was still skipping the image pushing part for some reason

@pohly
Copy link
Contributor Author

pohly commented Feb 25, 2019

Pull request builds don't push images. If they did, which tag should they use?

@pohly
Copy link
Contributor Author

pohly commented Feb 25, 2019

Wait, this repo is the one which doesn't have a Dockerfile, therefore image building is not enabled in the Makefile.

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants