Skip to content

Clusterctl integration#113

Merged
k8s-ci-robot merged 2 commits into
kubernetes-sigs:mainfrom
isaacdorfman:clusterctl-integration
May 10, 2022
Merged

Clusterctl integration#113
k8s-ci-robot merged 2 commits into
kubernetes-sigs:mainfrom
isaacdorfman:clusterctl-integration

Conversation

@isaacdorfman
Copy link
Copy Markdown

What this PR does / why we need it:
In order to use capk with clusterctl, i.e: "clusterctl init", "clusterctl generate", the github repo must contain a release with manifests in the form specified by the clusterctl.
https://cluster-api.sigs.k8s.io/clusterctl/provider-contract.html

This PR is intended to create a github action that creates such release each time a PR is merged into the main branch.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release notes:

@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 Mar 15, 2022
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 15, 2022
@coveralls
Copy link
Copy Markdown

coveralls commented Mar 15, 2022

Pull Request Test Coverage Report for Build 2301198525

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 45.93%

Totals Coverage Status
Change from base Build 2256884798: -0.1%
Covered Lines: 711
Relevant Lines: 1548

💛 - Coveralls

@davidvossel
Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 22, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2022
@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch from 21e9d88 to 95abf49 Compare April 25, 2022 12:03
@isaacdorfman isaacdorfman changed the title WIP: Clusterctl integration Clusterctl integration Apr 25, 2022
@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 Apr 25, 2022
@isaacdorfman
Copy link
Copy Markdown
Author

/retest

@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch from 95abf49 to ad3520e Compare April 25, 2022 12:55
Copy link
Copy Markdown
Contributor

@nirarg nirarg left a comment

Choose a reason for hiding this comment

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

Its look like this PR include also changes from image builder PR (#108)
Please change it to include only relevant changes to this PR

Comment thread .github/workflows/create_release.yaml Outdated
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread e2e/create-cluster_test.go Outdated
@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch 2 times, most recently from a0fd18c to 9a8e0a9 Compare April 27, 2022 11:45
@isaacdorfman
Copy link
Copy Markdown
Author

@davidvossel
I created a new makefile target called "functest-ubuntu" which uses environment variables that are relevant to the new image.
I need a way for the CI to invoke this target instead of the old one.

@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch 3 times, most recently from 56bcf15 to a744567 Compare May 2, 2022 10:03
Comment thread e2e/create-cluster_test.go Outdated
Comment thread config/manager/kustomization.yaml Outdated
@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch from a744567 to eb7e714 Compare May 2, 2022 12:18
Copy link
Copy Markdown
Contributor

@nirarg nirarg left a comment

Choose a reason for hiding this comment

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

You added two files named infrastructure-components.yaml, one under the root directory and one empty under config/manager
I guess this is mistake, please check

@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch 2 times, most recently from 1e703a7 to 16826aa Compare May 3, 2022 13:18
@nirarg
Copy link
Copy Markdown
Contributor

nirarg commented May 3, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2022
@davidvossel
Copy link
Copy Markdown
Contributor

related to #101

Comment thread infrastructure-components.yaml Outdated
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2022
@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch 3 times, most recently from 10271f2 to 0fcf17b Compare May 9, 2022 15:15
Copy link
Copy Markdown
Contributor

@davidvossel davidvossel left a comment

Choose a reason for hiding this comment

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

looks great, i just had some minor comments.

Comment thread controllers/kubevirtcluster_controller.go
Comment thread infrastructure-components.yaml Outdated
@@ -0,0 +1,5464 @@
apiVersion: v1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so we need to have this infrastructure-components.yaml in the source tree, or do we just need it uploaded as an asset to the release?

if we need it in the source tree, then we need something (a CI job) to ensure it's always up-to-date. If we don't need this yaml for the quickstart in the source tree itself, then i say we don't commit it and only upload it in it's generated form to the github release.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It only needs to be in the release.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deleted and gitignored it.

@davidvossel
Copy link
Copy Markdown
Contributor

/retest

@davidvossel
Copy link
Copy Markdown
Contributor

/hold

i'm trying to trigger e2e tests again.

@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 May 9, 2022
@davidvossel
Copy link
Copy Markdown
Contributor

davidvossel commented May 9, 2022

/hold cancel

for some reason holding and unholding triggers the e2e test due to the label change on the PR

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 9, 2022
@nirarg
Copy link
Copy Markdown
Contributor

nirarg commented May 10, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2022
@isaacdorfman isaacdorfman force-pushed the clusterctl-integration branch from 0fcf17b to 7f597e1 Compare May 10, 2022 14:08
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed 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. labels May 10, 2022
Copy link
Copy Markdown
Contributor

@davidvossel davidvossel left a comment

Choose a reason for hiding this comment

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

/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 May 10, 2022
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel, isaacdorfman

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

The pull request process is described here

Details 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 May 10, 2022
@k8s-ci-robot k8s-ci-robot merged commit 879a4cb into kubernetes-sigs:main May 10, 2022
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants