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.

Sign up for GitHub

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

Jump to bottom

Updates references to v1alpha4 types #1138

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Updates references to v1alpha4 types #1138

merged 1 commit into from
Mar 31, 2021

Conversation

srm09
Copy link
Contributor

@srm09 srm09 commented Mar 10, 2021 •
edited
Loading

What this PR does / why we need it:
The CAPI module in use now points to the latest on the master branch of CAPI. This patch updates the references from v1alpha3 to v1alpha4 types.
This patch has the following changes:

  • Use the latest master from the CAPI repo.
  • Updates controller-runtime to version v0.8.2
  • Updates golang version to v1.15.8
  • Updates the CAPV codebase to reference v1alpha4 types.
  • Uses the CAPI nightly builds for E2E tests with v1alpha4 types

Which issue(s) this PR fixes:
n/a

Special notes for your reviewer:
n/a

Release note:

Introduces support for v1alpha4 types

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 10, 2021
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 10, 2021
@yastij yastij self-assigned this Mar 10, 2021
@srm09
Copy link
Contributor Author

srm09 commented Mar 10, 2021

@yastij Does it make sense to break this into two separate PRs

  • One with the reference updates for v1alpha4
  • And the other one with just the test setup and the single test

@srm09
Copy link
Contributor Author

srm09 commented Mar 10, 2021

Re-arranged the commits to support breaking the PRs, if necessary

@srm09
Copy link
Contributor Author

srm09 commented Mar 10, 2021

PR to update golang version in the prow jobs: kubernetes/test-infra#21294

@srm09
Copy link
Contributor Author

srm09 commented Mar 11, 2021

/retitle Updates references to v1alpha4 types

@k8s-ci-robot k8s-ci-robot changed the title Adds support for controller tests Updates references to v1alpha4 types Mar 11, 2021
@srm09 srm09 mentioned this pull request Mar 11, 2021
Merged
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2021
srm09
srm09 commented Mar 16, 2021
View reviewed changes
Copy link
Contributor Author

@srm09 srm09 left a comment

Choose a reason for hiding this comment

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

Added some comments about a couple of changes

pkg/manager/manager.go Show resolved Hide resolved
pkg/manager/manager.go Show resolved Hide resolved
test/e2e/data/infrastructure-vsphere/kustomization/cluster-resource-set.yaml Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2021
@srm09
Copy link
Contributor Author

srm09 commented Mar 16, 2021

@yastij This is ready for review now.

gab-satchi
gab-satchi reviewed Mar 19, 2021
View reviewed changes
Copy link
Member

@gab-satchi gab-satchi left a comment

Choose a reason for hiding this comment

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

broadly lgtm. just minor nits

hack/tools/go.mod
k8s.io/code-generator v0.18.0
sigs.k8s.io/controller-tools v0.2.9
k8s.io/code-generator v0.20.2
sigs.k8s.io/controller-tools v0.4.1-0.20201002000720-57250aac17f6
Copy link
Member

Choose a reason for hiding this comment

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

does the controller-tools need to be the nightly build version as well? I see capi is using v0.5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, we are pulling in the CAPI sha which was before CAPI moved to go1.16 and k8s1.21. Hence controller-tools points to a nightly build. We might have another change to move forward to the latest CAPI version once an early build for version 0.4 gets released.

Copy link
Member

Choose a reason for hiding this comment

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

can we follow up on this with another PR ? @srm09

Copy link
Contributor Author

@srm09 srm09 Mar 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Opening an issue to track this one #1164

pkg/context/fake/fake_cluster_context.go Outdated Show resolved Hide resolved
pkg/context/fake/fake_controller_manager_context.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2021
yastij
yastij reviewed Mar 25, 2021
View reviewed changes
main.go Show resolved Hide resolved
test/e2e/config/vsphere-ci.yaml Show resolved Hide resolved
test/e2e/data/shared/metadata.yaml Show resolved Hide resolved
yastij
yastij approved these changes Mar 30, 2021
View reviewed changes
Copy link
Member

@yastij yastij left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yastij

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 Mar 30, 2021
@yastij
Copy link
Member

yastij commented Mar 30, 2021

/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 Mar 30, 2021
@srm09
Updates v1alpha3 refs to v1alpha4
9e74698 
- This patch updates the cluster-api dependency version to use the
latest from the main branch. As a result, the controller-runtime version
is also updates to v0.8.2
- This patch also updates the v1alpha3 refs in the codebase to v1alpha4.
- It also updates golang version to 1.15
- Changes to incorporate nightly CAPI builds

Signed-off-by: Sagar Muchhal <muchhals@vmware.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2021
@srm09
Copy link
Contributor Author

srm09 commented Mar 30, 2021

@yastij Squashed the commits
/unhold

@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 Mar 30, 2021
@yastij
Copy link
Member

yastij commented Mar 31, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2021
@k8s-ci-robot k8s-ci-robot merged commit 5553a61 into kubernetes-sigs:master Mar 31, 2021
@srm09 srm09 mentioned this pull request Apr 12, 2021
Merged
@srm09 srm09 mentioned this pull request Aug 18, 2021
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gab-satchi gab-satchi gab-satchi left review comments

@yastij yastij yastij approved these changes

@detiber detiber Awaiting requested review from detiber

@justinsb justinsb Awaiting requested review from justinsb

Assignees

@yastij yastij

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@srm09 @k8s-ci-robot @yastij @gab-satchi