add replaceBeforeVersion field for channel #8722

Ewocker · 2020-03-10T22:41:59Z

No description provided.

k8s-ci-robot · 2020-03-10T22:42:07Z

Hi @Ewocker. 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.

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.

k8s-ci-robot · 2020-03-10T22:43:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ewocker
To complete the pull request process, please assign gambol99
You can assign the PR to them by writing /assign @gambol99 in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

rifelpet · 2020-03-10T23:56:15Z

/ok-to-test

channels/pkg/api/channel.go

channels/pkg/channels/addon.go

channels/pkg/channels/addons_test.go

channels/pkg/channels/addon.go

channels/pkg/channels/apply.go

channels/pkg/channels/channel_version.go

johngmyers · 2020-03-10T23:57:07Z

channels/pkg/channels/channel_version.go

-func (c *ChannelVersion) replaces(existing *ChannelVersion) bool {
+// replaces returns 2 bool. First bool is true if new ChannelVersion if higher than the existing ChannelVersion.
+// Second bool is true if new ChannelVersion should force replace the existing ChannelVersion.
+func (c *ChannelVersion) replaces(existing *ChannelVersion) (bool, bool) {


Perhaps instead of returning two bools this should return a tri-valued type?

channels/pkg/api/channel.go

Ewocker · 2020-03-12T01:22:22Z

/retest

rifelpet · 2020-03-12T13:30:24Z

It looks like protokube is crashing in the e2e test:

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/kops/8722/pull-kops-e2e-kubernetes-aws/1237985818350456833/artifacts/13.211.154.224/protokube.log

I0312 06:29:36.080358    9623 s3fs.go:219] Reading file "s3://k8s-kops-prow/e2e-3106cff154-ff1eb.test-cncf-aws.k8s.io/addons/bootstrap-channel.yaml"
I0312 06:29:36.660577    9623 addons.go:129] Skipping version range "<1.12.0" that does not match current version 1.19.0
I0312 06:29:36.660602    9623 addons.go:129] Skipping version range "<1.12.0" that does not match current version 1.19.0
I0312 06:29:36.660610    9623 addons.go:129] Skipping version range "<1.15.0" that does not match current version 1.19.0
I0312 06:29:36.662650    9623 channel_version.go:108] Checking existing channel: <nil> compared to new channel: Version=1.5.0 Channel=s3://k8s-kops-prow/e2e-3106cff154-ff1eb.test-cncf-aws.k8s.io/addons/bootstrap-channel.yaml ManifestHash=2ea50e23f1a5aa41df3724630ac25173738cc90c
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x16ef913]
goroutine 1 [running]:
k8s.io/kops/channels/pkg/channels.(*ChannelVersion).updates(0xc0007c1300, 0x0, 0xc000458b40)
	channels/pkg/channels/channel_version.go:109 +0x93
k8s.io/kops/channels/pkg/channels.(*Addon).GetRequiredUpdates(0xc00043cf20, 0x20bc6e0, 0xc000458b40, 0x0, 0x0, 0x0)
	channels/pkg/channels/addon.go:100 +0x17f
k8s.io/kops/channels/pkg/cmd.RunApplyChannel(0x203ef60, 0xc000449010, 0x2041060, 0xc0000aa008, 0xc000462de0, 0xc000474600, 0x1, 0x3, 0x0, 0x0)
	channels/pkg/cmd/apply_channel.go:136 +0x609
k8s.io/kops/channels/pkg/cmd.NewCmdApplyChannel.func1(0xc00047c000, 0xc000474600, 0x1, 0x3, 0x0, 0x0)
	channels/pkg/cmd/apply_channel.go:44 +0x70
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).execute(0xc00047c000, 0xc000474570, 0x3, 0x3, 0xc00047c000, 0xc000474570)
	vendor/github.com/spf13/cobra/command.go:826 +0x460
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0xc000465900, 0x2fc90e0, 0x0, 0x0)
	vendor/github.com/spf13/cobra/command.go:914 +0x2fb
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).Execute(...)
	vendor/github.com/spf13/cobra/command.go:864
k8s.io/kops/channels/pkg/cmd.Execute(0x203ef60, 0xc000449010, 0x2041060, 0xc0000aa008, 0xc000371f50, 0x40576f)
	channels/pkg/cmd/root.go:40 +0x1ac
main.main()
	channels/cmd/channels/main.go:31 +0x66

Ewocker · 2020-03-12T17:41:26Z

@rifelpet Thanks.
Is there anyway I can check the namespace status of the e2e test (want to see the version annotation) because the nil pointer does not seem likely to be a cause related to my change (although it may be). I am not too sure about the e2e test flow, but when using the dev-build.sh, I saw multiple errors in kubelet where it says is forbidden: User "kubelet" cannot get resource. Not sure if it can be related.

channels/pkg/channels/addon.go

johngmyers · 2020-05-10T04:41:35Z

Per Kops Office Hours (though taken from memory) @justinsb to do some checking with other sections of the Kubernetes organization.

k8s-ci-robot · 2020-05-10T04:41:42Z

@Ewocker: PR needs rebase.

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.

olemarkus · 2020-06-03T10:28:31Z

What is the status of this one now?

rifelpet · 2020-06-03T13:04:21Z

go.mod

@@ -53,7 +53,6 @@ require (
 	github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd
 	github.com/Masterminds/semver v1.3.1 // indirect
 	github.com/Masterminds/sprig v2.17.1+incompatible
-	github.com/Microsoft/go-winio v0.4.14 // indirect


can you remove these go.* changes? the rest of the code isn't changing any imports so I'm not sure why these would need to be removed. They're also causing a git conflict so removing the changes is probably the easiest resolution.

rifelpet · 2020-06-03T13:07:16Z

@olemarkus I think we're waiting on a review from @justinsb

once this lands we should consider using it with #7919 if needed.

/assign @justinsb

k8s-ci-robot · 2021-01-11T11:53:42Z

@Ewocker: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kops-verify-cloudformation	`2e57fe9`	link	`/test pull-kops-verify-cloudformation`
pull-kops-verify-hashes	`2e57fe9`	link	`/test pull-kops-verify-hashes`
pull-kops-e2e-k8s-containerd	`2e57fe9`	link	`/test pull-kops-e2e-k8s-containerd`

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.

fejta-bot · 2021-04-11T13:21:03Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

fejta-bot · 2021-05-11T13:22:57Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

fejta-bot · 2021-06-10T13:52:43Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

k8s-ci-robot · 2021-06-10T13:52:49Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

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.

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 10, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 10, 2020

k8s-ci-robot requested review from joshbranham and robinpercy March 10, 2020 22:43

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 Mar 10, 2020

johngmyers requested changes Mar 11, 2020

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 11, 2020

rifelpet reviewed Mar 12, 2020

View reviewed changes

channels/pkg/channels/addon.go Outdated Show resolved Hide resolved

Ewocker marked this pull request as ready for review March 12, 2020 23:19

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 12, 2020

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 19, 2020

Ewocker closed this Mar 20, 2020

Ewocker reopened this Mar 20, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2020

add replaceBeforeVersion field for channel

2e57fe9

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2020

rifelpet reviewed Jun 3, 2020

View reviewed changes

k8s-ci-robot assigned justinsb Jun 3, 2020

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 11, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 11, 2021

k8s-ci-robot closed this Jun 10, 2021

johngmyers mentioned this pull request Jun 16, 2021

add replaceBeforeVersion field for channel #11776

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add replaceBeforeVersion field for channel #8722

add replaceBeforeVersion field for channel #8722

Ewocker commented Mar 10, 2020

k8s-ci-robot commented Mar 10, 2020

k8s-ci-robot commented Mar 10, 2020

rifelpet commented Mar 10, 2020

johngmyers Mar 10, 2020

Ewocker commented Mar 12, 2020

rifelpet commented Mar 12, 2020

Ewocker commented Mar 12, 2020 •

edited

Loading

johngmyers commented May 10, 2020

k8s-ci-robot commented May 10, 2020

olemarkus commented Jun 3, 2020

rifelpet Jun 3, 2020

rifelpet commented Jun 3, 2020

k8s-ci-robot commented Jan 11, 2021

fejta-bot commented Apr 11, 2021

fejta-bot commented May 11, 2021

fejta-bot commented Jun 10, 2021

k8s-ci-robot commented Jun 10, 2021

add replaceBeforeVersion field for channel #8722

add replaceBeforeVersion field for channel #8722

Conversation

Ewocker commented Mar 10, 2020

k8s-ci-robot commented Mar 10, 2020

k8s-ci-robot commented Mar 10, 2020

rifelpet commented Mar 10, 2020

johngmyers Mar 10, 2020

Choose a reason for hiding this comment

Ewocker commented Mar 12, 2020

rifelpet commented Mar 12, 2020

Ewocker commented Mar 12, 2020 • edited Loading

johngmyers commented May 10, 2020

k8s-ci-robot commented May 10, 2020

olemarkus commented Jun 3, 2020

rifelpet Jun 3, 2020

Choose a reason for hiding this comment

rifelpet commented Jun 3, 2020

k8s-ci-robot commented Jan 11, 2021

fejta-bot commented Apr 11, 2021

fejta-bot commented May 11, 2021

fejta-bot commented Jun 10, 2021

k8s-ci-robot commented Jun 10, 2021

Ewocker commented Mar 12, 2020 •

edited

Loading