⚠️CABPK - Create v1alpha3 types #1800

noamran · 2019-11-25T23:37:19Z

What this PR does / why we need it:
Creates the v1alpha3 types for the kubeadm bootstrap provider.
Which issue(s) this PR fixes:
Fixes #1783 and #1823

k8s-ci-robot · 2019-11-25T23:37:27Z

Hi @noamran. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

vincepri · 2019-11-26T16:22:08Z

/ok-to-test

fabriziopandini · 2019-11-26T20:50:24Z

bootstrap/kubeadm/api/v1alpha3/types.go

+package v1alpha3
+
+// KubeadmConfigTemplateResource defines the Template structure
+type KubeadmConfigTemplateResource struct {


Is there a reason to keep this into a separated file? (vs merging into kubeadmconfigtemplate_types.go)

Not sure... I can move it to kubeadmconfigtemplate_types.go if there are no objections.

ncdc · 2019-12-03T19:06:55Z

/lgtm
/approve

k8s-ci-robot · 2019-12-03T19:07:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc, noamran

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:

~~OWNERS~~ [ncdc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

detiber · 2019-12-03T19:28:08Z

Looks like a rebase is needed.

noamran · 2019-12-03T19:29:54Z

/hold
Resolving conflicts and adding the conversion webhooks.

noamran · 2019-12-05T00:19:32Z

/hold cancel

detiber · 2019-12-05T01:55:03Z

main.go

+			setupLog.Error(err, "unable to create webhook", "webhook", "KubeadmConfigTemplate")
+			os.Exit(1)
+		}
+		if err = (&kubeadmbootstrapv1alpha3.KubeadmConfigTemplateList{}).SetupWebhookWithManager(mgr); err != nil {


It looks like for the other conversion webhooks we are setting up the webhook through the v1alpha2 types, but the approach here feels more correct to me (since v1alpha3 is being used as the hub type). I'm wondering if we need to change the other conversion webhooks to be configured through the v1alpha3 types as well.

IMO it would make more sense to have all the conversion webhooks configured through the same hub type version (v1alpha3 in this case).

vincepri · 2019-12-02T20:24:23Z

bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go

+	NTP *NTP `json:"ntp,omitempty"`
+	// Format specifies the output format of the bootstrap data
+	// +optional
+	Format Format `json:"format,omitempty"`


Nit: leave a space between each field and the next comment, it helps with readability (and it's consistent with the other structs in this file)

vincepri

/lgtm

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 25, 2019

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 25, 2019

k8s-ci-robot requested review from chuckha and fabriziopandini November 25, 2019 23:37

noamran changed the title ~~⚠️CABPK -Create v1alpha3 types~~ [WIP] ⚠️CABPK -Create v1alpha3 types Nov 25, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2019

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 26, 2019

noamran changed the title ~~[WIP] ⚠️CABPK -Create v1alpha3 types~~ [WIP] ⚠️CABPK - Create v1alpha3 types Nov 26, 2019

fabriziopandini reviewed Nov 26, 2019

View reviewed changes

noamran force-pushed the 1783 branch 2 times, most recently from 91f119b to c986934 Compare November 27, 2019 22:52

noamran changed the title ~~[WIP] ⚠️CABPK - Create v1alpha3 types~~ ⚠️CABPK - Create v1alpha3 types Dec 2, 2019

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 Dec 2, 2019

noamran mentioned this pull request Dec 3, 2019

📖 Renaming ErrorMessage and ErrorReason to FailureMessage and FailureReason in docs #1822

Merged

ncdc self-assigned this Dec 3, 2019

ncdc added this to the v0.3.0 milestone Dec 3, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2019

noamran mentioned this pull request Dec 3, 2019

Conversion webhooks for CABPK v1alpha2 to v1alpha3 #1823

Closed

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Dec 3, 2019

noamran changed the title ~~⚠️CABPK - Create v1alpha3 types~~ [WIP] ⚠️CABPK - Create v1alpha3 types Dec 4, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2019

noamran force-pushed the 1783 branch from cb31001 to 3bebaa5 Compare December 4, 2019 02:43

CABPK - v1alpha3 types

68dee24

noamran force-pushed the 1783 branch from 3bebaa5 to 68dee24 Compare December 4, 2019 02:53

noamran changed the title ~~[WIP] ⚠️CABPK - Create v1alpha3 types~~ ⚠️CABPK - Create v1alpha3 types Dec 4, 2019

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 Dec 4, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 5, 2019

detiber reviewed Dec 5, 2019

View reviewed changes

vincepri reviewed Dec 5, 2019

View reviewed changes

vincepri approved these changes Dec 5, 2019

View reviewed changes

k8s-ci-robot assigned vincepri Dec 5, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2019

k8s-ci-robot merged commit 91efb1d into kubernetes-sigs:master Dec 5, 2019

noamran deleted the 1783 branch January 9, 2020 18:09

noamran mentioned this pull request Jan 22, 2020

REQUEST: New membership for noamran kubernetes/org#1587

Closed

6 tasks

⚠️CABPK - Create v1alpha3 types #1800

⚠️CABPK - Create v1alpha3 types #1800

Uh oh!

Conversation

noamran commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 25, 2019

Uh oh!

vincepri commented Nov 26, 2019

Uh oh!

fabriziopandini Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

noamran Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

ncdc commented Dec 3, 2019

Uh oh!

k8s-ci-robot commented Dec 3, 2019

Uh oh!

detiber commented Dec 3, 2019

Uh oh!

noamran commented Dec 3, 2019

Uh oh!

noamran commented Dec 5, 2019

Uh oh!

detiber Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

noamran Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

vincepri Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

vincepri left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

noamran commented Nov 25, 2019 •

edited

Loading