fix --local panic in set commands by juanvallejo · Pull Request #65636 · kubernetes/kubernetes

juanvallejo · 2018-06-29T14:38:31Z

Release note:

Fixed panic when performing a "set env" operation on a --local resource

~~In progress - identifying other commands with this issue~~

cc @soltysh

soltysh

/lgtm
/approve

liggitt · 2018-06-29T15:47:27Z

pkg/kubectl/cmd/set/set_env.go

kind is not the same as resource... this changes the output here

Good point.

Since we were panicing on --local before (hence no output to change under that code path), I can go ahead and leave info.Mapping.Resource as it was, and only output kind under --local. wdyt?

That's a reasonable middle ground, eventually we can entirely skip that part of the message with --local

I guess we can do the same thing we do for -o name without resource info... lower(<kind>[.<group>])

Thanks, PR updated

juanvallejo · 2018-06-29T17:20:02Z

/hold

idealhack · 2018-06-30T06:31:22Z

/test pull-kubernetes-e2e-gce

juanvallejo · 2018-07-12T14:59:04Z

/test pull-kubernetes-e2e-kops-aws

juanvallejo · 2018-07-23T18:43:46Z

@liggitt friendly ping

liggitt · 2018-07-26T18:12:55Z

pkg/kubectl/cmd/util/helpers.go

any reason not to make this a method on resource.Info, similar to it's existing String() method?

I'll open a followup to address that.

soltysh · 2018-08-15T09:07:55Z

@juanvallejo please address Jordan's comments.

nikhita · 2018-09-25T09:06:50Z

/sig cli
/kind bug

juanvallejo · 2018-09-25T22:31:55Z

@soltysh

please address Jordan's comments.

Sorry about my delay here. Will find some time to wrap this PR up this week

soltysh · 2019-01-15T14:05:20Z

@juanvallejo friendly ping

juanvallejo · 2019-02-06T21:51:54Z

/test pull-kubernetes-integration

soltysh

/lgtm
/approve

soltysh · 2019-02-07T11:44:43Z

pkg/kubectl/cmd/util/helpers.go

I'll open a followup to address that.

soltysh

/lgtm

k8s-ci-robot · 2019-02-07T22:19:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juanvallejo, soltysh

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:

~~pkg/kubectl/OWNERS~~ [soltysh]
~~staging/src/k8s.io/cli-runtime/OWNERS~~ [juanvallejo,soltysh]

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

fejta-bot · 2019-02-08T01:28:17Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 29, 2018

juanvallejo changed the title ~~fix --local panic "set env"~~ [WIP] fix --local panic "set env" Jun 29, 2018

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 Jun 29, 2018

k8s-ci-robot requested review from mengqiy and smarterclayton June 29, 2018 14:38

juanvallejo changed the title ~~[WIP] fix --local panic "set env"~~ fix --local panic in set commands Jun 29, 2018

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 Jun 29, 2018

juanvallejo force-pushed the jvallejo/fix-local-panic branch from 098122a to e1cbe7f Compare June 29, 2018 14:46

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 29, 2018

juanvallejo force-pushed the jvallejo/fix-local-panic branch 2 times, most recently from de28d7d to c821f1d Compare June 29, 2018 15:05

soltysh approved these changes Jun 29, 2018

View reviewed changes

k8s-ci-robot assigned soltysh Jun 29, 2018

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2018

soltysh added cherrypick-candidate and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2018

juanvallejo mentioned this pull request Jun 29, 2018

Automated cherry pick of #65636: fix --local panic in set commands #65639

Closed

liggitt reviewed Jun 29, 2018

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 29, 2018

juanvallejo force-pushed the jvallejo/fix-local-panic branch from c821f1d to 40df4e5 Compare June 29, 2018 17:24

liggitt reviewed Jul 26, 2018

View reviewed changes

k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. and removed cherrypick-candidate labels Sep 21, 2018

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

juanvallejo force-pushed the jvallejo/fix-local-panic branch from a4fc40a to 099f987 Compare February 6, 2019 20:54

k8s-ci-robot added the area/kubectl label Feb 6, 2019

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

soltysh approved these changes Feb 7, 2019

View reviewed changes

pkg/kubectl/cmd/util/helpers.go Outdated

Copy link

Contributor

soltysh Feb 7, 2019

Choose a reason for hiding this comment

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

I'll open a followup to address that.

juanvallejo force-pushed the jvallejo/fix-local-panic branch from 74e3d9c to bded018 Compare February 7, 2019 21:51

fix --local panic in set commands

e8df247

juanvallejo force-pushed the jvallejo/fix-local-panic branch from bded018 to e8df247 Compare February 7, 2019 21:56

soltysh approved these changes Feb 7, 2019

View reviewed changes

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 7, 2019

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

k8s-ci-robot merged commit 836ce9f into kubernetes:master Feb 8, 2019

Conversation

juanvallejo commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juanvallejo commented Jun 29, 2018

Uh oh!

idealhack commented Jun 30, 2018

Uh oh!

juanvallejo commented Jul 12, 2018

Uh oh!

juanvallejo commented Jul 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh commented Aug 15, 2018

Uh oh!

nikhita commented Sep 25, 2018

Uh oh!

juanvallejo commented Sep 25, 2018

Uh oh!

soltysh commented Jan 15, 2019

Uh oh!

juanvallejo commented Feb 6, 2019

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Feb 7, 2019

Uh oh!

fejta-bot commented Feb 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

juanvallejo commented Jun 29, 2018 •

edited

Loading