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 artifacts MVP #580

Merged
merged 9 commits into from
Feb 20, 2019

Conversation

randomvariable
Copy link
Member

@randomvariable randomvariable commented Feb 14, 2019

What this PR does / why we need it:
Refactors the way manifests are generated so that provider components are generated by Bazel in line with the image.

make release-artifacts will generate a tar file with the manifest templates, shell script and getting started guide.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #578
Fixes #569,
Related to #549, #276, #122

Special notes for your reviewer:

E2E tests were rewired to make use of the artifact creation here, so that it tests the manifests generated by the tools we provide to the user. During that, made some changes to provide more logging information during the run. Can confirm no secrets are leaked by doing this.

Release note:

Sample manifests are now provided as a tar file with each release.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 14, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: randomvariable

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 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. labels Feb 14, 2019
@randomvariable
Copy link
Member Author

Back.

I'll need to rebase onto #573

@randomvariable randomvariable changed the title [wip] Release artifacts MVP Release artifacts MVP Feb 15, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 15, 2019
@randomvariable
Copy link
Member Author

Ready for review.

@randomvariable
Copy link
Member Author

randomvariable commented Feb 15, 2019

cc @detiber @chuckha @vincepri

Apologies for the size. Needed to unpick how manifests are generated and create separate pathways in bazel for release, dev, and e2e testing.

Copy link
Contributor

@chuckha chuckha left a comment

Choose a reason for hiding this comment

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

This is great work. I think I need a little more time to digest, but initial pass is done

cmd/clusterctl/examples/aws/generate-yaml.sh Outdated Show resolved Hide resolved
cmd/clusterctl/examples/aws/generate-yaml.sh Outdated Show resolved Hide resolved
test/integration/BUILD Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
cmd/clusterctl/examples/aws/BUILD.bazel Outdated Show resolved Hide resolved
cmd/clusterctl/examples/aws/BUILD.bazel Outdated Show resolved Hide resolved
config/manager/manager.yaml Outdated Show resolved Hide resolved
docs/getting-started.md Show resolved Hide resolved
test/e2e/util/kind/setup.go Outdated Show resolved Hide resolved
Signed-off-by: Naadir Jeewa <[email protected]>
@randomvariable
Copy link
Member Author

Cleaned this up a little more following @chuckha's recommendations.

Copy link
Contributor

@chuckha chuckha left a comment

Choose a reason for hiding this comment

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

This looks good to me, I have one question and then will give it a day to see if anyone else has more feedback

build/stateful_set_patch.bzl Outdated Show resolved Hide resolved
Copy link
Contributor

@chuckha chuckha left a comment

Choose a reason for hiding this comment

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

/lgtm

A+

@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
No longer use a variable lookup in the rule.

Signed-off-by: Naadir Jeewa <[email protected]>
@randomvariable
Copy link
Member Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2019
@randomvariable
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2019
@randomvariable
Copy link
Member Author

/hold cancel

@chuckha
Copy link
Contributor

chuckha commented Feb 20, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 20, 2019
@k8s-ci-robot k8s-ci-robot merged commit e6bb385 into kubernetes-sigs:master Feb 20, 2019
@randomvariable randomvariable deleted the 010-artifacts branch February 20, 2019 21:50
@k8s-ci-robot
Copy link
Contributor

@randomvariable: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-cluster-api-provider-aws-bazel-e2e f195a26 link /test pull-cluster-api-provider-aws-bazel-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

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.

Update manifests and docs for v0.1.0 Kustomize 2.0+ not compatible with getting started guide
3 participants