add apps/v1beta2 conversion tests #49645

dixudx · 2017-07-26T16:17:08Z

What this PR does / why we need it:

add apps/v1beta2 conversion test

Depend on ~~#49751~~(Merged), ~~#49719~~(Merged)

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:
/cc @janetkuo

Release note:

add apps/v1beta2 conversion test

k8s-ci-robot · 2017-07-26T16:17:16Z

Hi @dixudx. Thanks for your PR.

I'm waiting for a kubernetes 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.

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. I understand the commands that are listed here.

dixudx · 2017-07-27T01:24:07Z

@janetkuo As you said, there is no need to add conversion_test.go now. I think I can close this PR.

janetkuo · 2017-07-27T07:21:30Z

@k8s-bot ok to test

janetkuo · 2017-07-27T07:23:08Z

@dixudx it's still nice to have unit tests for those conversion codes. I will review this PR once it's ready

dixudx · 2017-07-27T07:25:36Z

@janetkuo Thanks. Also I found some conversion bugs during testing these tests. I think we would address other PRs to fix this.

dixudx · 2017-07-28T16:20:27Z

@janetkuo PTAL. Thanks.

dixudx · 2017-08-25T12:56:07Z

ping @janetkuo

PTAL. This is quite important for apps/v1beta2.

dixudx · 2017-08-28T15:13:10Z

/assign @kow3ns @liggitt

PTAL. Thanks.

janetkuo · 2017-08-28T18:06:33Z

pkg/apis/apps/v1beta2/defaults.go

@@ -126,3 +126,9 @@ func SetDefaults_ReplicaSet(obj *appsv1beta2.ReplicaSet) {
 		*obj.Spec.Replicas = 1
 	}
 }
+
+func SetDefaults_RollingUpdateDaemonSet(obj *appsv1beta2.RollingUpdateDaemonSet) {


This is not called by anyone?

Updated generated files. Done. PTAL.

janetkuo · 2017-08-28T18:11:02Z

pkg/apis/apps/v1beta2/conversion.go

@@ -118,6 +118,9 @@ func Convert_extensions_RollingUpdateDaemonSet_To_v1beta2_RollingUpdateDaemonSet
 }

 func Convert_v1beta2_RollingUpdateDaemonSet_To_extensions_RollingUpdateDaemonSet(in *appsv1beta2.RollingUpdateDaemonSet, out *extensions.RollingUpdateDaemonSet, s conversion.Scope) error {
+	if in.MaxUnavailable == nil {


It can't be nil (and its value has to be >0)

This should be removed, as appsv1beta2.RollingUpdateDaemonSet is never nil (based on default and validation).

janetkuo · 2017-08-28T18:11:56Z

/retest

janetkuo · 2017-08-29T18:39:43Z

pkg/apis/apps/v1beta2/zz_generated.defaults.go

@@ -176,6 +176,9 @@ func SetObjectDefaults_DaemonSet(in *v1beta2.DaemonSet) {
 			}
 		}
 	}
+	if in.Spec.UpdateStrategy.RollingUpdate != nil {
+		SetDefaults_RollingUpdateDaemonSet(in.Spec.UpdateStrategy.RollingUpdate)


This should be removed.

SetDefaults_DaemonSet was called in L45 which sets default RollingUpdateDaemonSet already (and sets MaxUnavailable to 1).

@janetkuo Updated. PTAL.

janetkuo · 2017-08-29T18:41:20Z

pkg/apis/apps/v1beta2/conversion.go

@@ -118,6 +118,9 @@ func Convert_extensions_RollingUpdateDaemonSet_To_v1beta2_RollingUpdateDaemonSet
 }

 func Convert_v1beta2_RollingUpdateDaemonSet_To_extensions_RollingUpdateDaemonSet(in *appsv1beta2.RollingUpdateDaemonSet, out *extensions.RollingUpdateDaemonSet, s conversion.Scope) error {
+	if in.MaxUnavailable == nil {


This should be removed, as appsv1beta2.RollingUpdateDaemonSet is never nil (based on default and validation).

janetkuo · 2017-08-30T01:10:44Z

/retest

janetkuo · 2017-08-30T22:37:52Z

pkg/apis/apps/v1beta2/conversion_test.go

+			t.Errorf("%q - %q: unexpected error: %v", k, "from extensions to v1beta2", err)
+		}
+
+		if !apiequality.Semantic.DeepDerivative(internal1, tc.stsSepc2) {


Use apiequality.Semantic.DeepEqual instead of DeepDerivative everywhere in this PR.

@janetkuo Updated. PTAL. Thanks.

janetkuo · 2017-08-30T22:41:36Z

pkg/apis/apps/v1beta2/conversion_test.go

+		}
+		if tc.adjustment {
+			if tc.stsSpec1.Template.Spec.SecurityContext == nil {
+				tc.stsSpec1.Template.Spec.SecurityContext = new(api.PodSecurityContext)


Why do we need this?

janetkuo · 2017-08-31T05:27:05Z

/lgtm

k8s-ci-robot · 2017-08-31T06:41:52Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel	`55c2a3e`	link	`/test pull-kubernetes-bazel`

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.

janetkuo · 2017-08-31T22:30:09Z

/lgtm

dixudx · 2017-09-01T00:57:16Z

ping @liggitt

PTAL. Need your approval. Thanks.

liggitt · 2017-09-01T01:53:37Z

/approve no-issue

k8s-github-robot · 2017-09-01T01:54:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, janetkuo, liggitt

Associated issue: 49751

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

Needs approval from an approver in each of these OWNERS Files:

~~pkg/apis/OWNERS~~ [liggitt]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-09-01T23:34:00Z

Automatic merge from submit-queue (batch tested with PRs 50381, 51307, 49645, 50995, 51523)

k8s-ci-robot requested a review from janetkuo July 26, 2017 16:17

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 Jul 26, 2017

k8s-github-robot assigned smarterclayton and pmorie Jul 26, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 26, 2017

dixudx mentioned this pull request Jul 27, 2017

Cleanup DaemonSet templateGeneration #49454

Merged

janetkuo assigned janetkuo and unassigned pmorie and smarterclayton Jul 27, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 27, 2017

janetkuo added this to the v1.8 milestone Jul 27, 2017

dixudx mentioned this pull request Jul 27, 2017

fix apps DeploymentSpec conversion issue #49719

Merged

dixudx changed the title ~~[WIP] add V1beta2 conversion tests~~ add apps/v1beta2 conversion tests Jul 28, 2017

dixudx force-pushed the v1beta2_conversion_test branch from 838dc54 to b3d9471 Compare July 28, 2017 16:17

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 28, 2017

dixudx force-pushed the v1beta2_conversion_test branch 2 times, most recently from 5e22984 to 0b98c42 Compare July 29, 2017 06:51

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2017

dixudx force-pushed the v1beta2_conversion_test branch from 0b98c42 to a282a91 Compare August 8, 2017 14:04

k8s-github-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 8, 2017

k8s-ci-robot assigned kow3ns and liggitt Aug 28, 2017

janetkuo reviewed Aug 28, 2017

View reviewed changes

dixudx force-pushed the v1beta2_conversion_test branch from 55c2a3e to 8e78bd4 Compare August 29, 2017 03:05

janetkuo reviewed Aug 29, 2017

View reviewed changes

dixudx force-pushed the v1beta2_conversion_test branch from 8e78bd4 to 1ca59e6 Compare August 30, 2017 14:50

janetkuo reviewed Aug 30, 2017

View reviewed changes

dixudx force-pushed the v1beta2_conversion_test branch 2 times, most recently from 5349401 to fac112f Compare August 31, 2017 02:57

janetkuo approved these changes Aug 31, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2017

dixudx force-pushed the v1beta2_conversion_test branch from fac112f to 02daf88 Compare August 31, 2017 08:11

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2017

add tests for apps/v1beta2 conversion

a76f538

dixudx force-pushed the v1beta2_conversion_test branch from 02daf88 to a76f538 Compare August 31, 2017 08:25

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2017

k8s-github-robot merged commit 5f405b7 into kubernetes:master Sep 1, 2017

dixudx deleted the v1beta2_conversion_test branch September 2, 2017 01:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add apps/v1beta2 conversion tests #49645

add apps/v1beta2 conversion tests #49645

dixudx commented Jul 26, 2017 •

edited

Loading

k8s-ci-robot commented Jul 26, 2017

dixudx commented Jul 27, 2017

janetkuo commented Jul 27, 2017

janetkuo commented Jul 27, 2017

dixudx commented Jul 27, 2017

dixudx commented Jul 28, 2017

dixudx commented Aug 25, 2017

dixudx commented Aug 28, 2017

janetkuo Aug 28, 2017

dixudx Aug 29, 2017

janetkuo Aug 28, 2017

janetkuo Aug 29, 2017

janetkuo commented Aug 28, 2017

janetkuo Aug 29, 2017

dixudx Aug 30, 2017

janetkuo Aug 29, 2017

janetkuo commented Aug 30, 2017

janetkuo Aug 30, 2017

dixudx Aug 31, 2017

janetkuo Aug 30, 2017

janetkuo commented Aug 31, 2017

k8s-ci-robot commented Aug 31, 2017 •

edited

Loading

janetkuo commented Aug 31, 2017

dixudx commented Sep 1, 2017

liggitt commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

add apps/v1beta2 conversion tests #49645

add apps/v1beta2 conversion tests #49645

Conversation

dixudx commented Jul 26, 2017 • edited Loading

k8s-ci-robot commented Jul 26, 2017

dixudx commented Jul 27, 2017

janetkuo commented Jul 27, 2017

janetkuo commented Jul 27, 2017

dixudx commented Jul 27, 2017

dixudx commented Jul 28, 2017

dixudx commented Aug 25, 2017

dixudx commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janetkuo commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janetkuo commented Aug 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janetkuo commented Aug 31, 2017

k8s-ci-robot commented Aug 31, 2017 • edited Loading

janetkuo commented Aug 31, 2017

dixudx commented Sep 1, 2017

liggitt commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

k8s-github-robot commented Sep 1, 2017

dixudx commented Jul 26, 2017 •

edited

Loading

k8s-ci-robot commented Aug 31, 2017 •

edited

Loading