Re: [kubernetes/kubernetes] Some edits can drop users out of kubectl edit (#42217)

[Clayton Coleman]

Unsure if this is a regression @kubernetes/sig-cli-bugs

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[Phillip Wittrock]

@ymqytw Can you take a look at this?

[Mengqi Yu]

Should we quote the number?

env:
- name: A
  value: "1"
- name: B
  value: "2"

[Phillip Wittrock]

I wonder if this is related to using UnstructuredObject in edit instead of round tripping through go structs. I am not sure how the yaml could be converted to json in an unambiguous way without the type schema.

[Phillip Wittrock]

@smarterclayton I think maybe it shouldn't give a validation error, and instead should just work.

[Mengqi Yu]

This is not a regression. Same issue happens in 1.4.9 and 1.5.3.

[Phillip Wittrock]

@ymqytw Thanks for checking on that. Lets start thinking about a solution.

[Clayton Coleman]

So I want to move JSON validation to the server side, and we probably want
patch to respond with an an invalid structural error when the types don't
match (it should return 422 Invalid if the patch itself is structurally
unsound).

On Tue, Feb 28, 2017 at 5:09 PM, Phillip Wittrock <notifi...@github.com>
wrote:

> @ymqytw <https://github.com/ymqytw> Thanks for checking on that. Lets

> start thinking about a solution.
>
> —

> You are receiving this because you were mentioned.

> Reply to this email directly, view it on GitHub

> <https://github.com/kubernetes/kubernetes/issues/42217#issuecomment-283177116>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABG_pyFM0g13F1DD75KieCcozjLDyw9pks5rhJsagaJpZM4MN6lN>

[Phillip Wittrock]

@smarterclayton Moving validation to server side sounds like a good general solution. Do you have preference for what we do (if anything) for 1.6?

[Clayton Coleman]

I don't know that this is is important enough to fix for 1.6 since it's
fairly complicated to get right. I don't think the incremental fix helps
enough people to justify the cost.

On Tue, Feb 28, 2017 at 7:28 PM, Phillip Wittrock <notifi...@github.com>
wrote:

> @smarterclayton <https://github.com/smarterclayton> Moving validation to

> server side sounds like a good general solution. Do you have preference for
> what we do (if anything) for 1.6?
>
> —

> You are receiving this because you were mentioned.

> Reply to this email directly, view it on GitHub

> <https://github.com/kubernetes/kubernetes/issues/42217#issuecomment-283205656>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABG_pygY907nZvsKKRLQx6xOvnN9X40Dks5rhLucgaJpZM4MN6lN>
> .

[Mengqi Yu]

Should we remove the 1.6 milestone?

[Clayton Coleman]

I'm fine with that

[Beni Cherniavsky-Paskin]

see also related #26050

[Aaron Crickenberger]

/remove-priority P1
removing old priority/PN labels, this already has priority/important-soon

[fejta-bot]

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.

Prevent issues from auto-closing with an /lifecycle frozen comment.

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

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

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-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

[k8s-ci-robot]

Closed #42217.

[fejta-bot]

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-testing, kubernetes/test-infra and/or fejta.
/close

[Davanum Srinivas]

Reopened #42217.

[Davanum Srinivas]

/lifecycle frozen

[Brian Pursley]

/kind bug

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or unsubscribe.

[Brian Pursley]

I think this might be fixed now.

Using the steps in the issue description, when I save the changes, it immediately reloads vim with the following message at the top:

#
# pods "nginx" was not valid:
# * patch: Invalid value: "map[spec:map[]]": cannot convert int64 to string
#

I tried several versions of kubectl back to 1.15.11, so I'm not sure at what point this might have been fixed.

[Maciej Szulik]

As Brian put it it's working as expected.
/close

[Kubernetes Prow Robot]

Closed #42217.

[Kubernetes Prow Robot]

@soltysh: Closing this issue.

In response to this:

As Brian put it it's working as expected.
/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.