Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Umbrella Issue for Box-affecting issues #39900

Closed
ahakanbaba opened this issue Jan 14, 2017 · 19 comments
Closed

Umbrella Issue for Box-affecting issues #39900

ahakanbaba opened this issue Jan 14, 2017 · 19 comments
Assignees
Labels
needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one.

Comments

@ahakanbaba
Copy link
Contributor

ahakanbaba commented Jan 14, 2017

@bgrant0607
Copy link
Member

cc @pwittrock @ymqytw @kow3ns

@liggitt
Copy link
Member

liggitt commented Jan 21, 2017

kubectl patch computation for TPR has never worked correctly (either in the patch computation or in what is submitted to the server). I would recommend you use kubectl update with third party resources until kubectl supports them properly

@mengqiy
Copy link
Member

mengqiy commented Jan 31, 2017

@ahakanbaba I have a PR #40666 for TPR. I think it will make apply works with TPR.
Because it is based on #40260, #40666 may not be back ported to 1.4 and 1.5. Are you OK?

@ahakanbaba
Copy link
Contributor Author

Hi @ymqytw
Not having this in 1.5 is a big problem for us. On our side we will need to do several changes.

I see that the #40666 is quite a big change.
How reasonable is it to port it to 1.5?

We extensively use the apply command on TPR's but because of various known bugs we only really have two use cases:

  1. Creation
  2. Idempotent apply without any change.

In other words we do not modify existing TPR entries. However an apply command may re-execute on the same json file without any changes. (Idempotent) We have converged to this state after several workarounds on our side.

Up until 1.4.7 these have been working fine.

You can imagine this makes our implementation simple while using kubectl. We only rely on apply.
It is undesirable to add one more workaround to avoid apply for TPR's while requiring it for other resource types. If we do not modify 1.5, it looks like we need to do that.

In our experience in 1.4.8 the Creation use case was working fine. Our problem was with idempotent apply operations.

How difficult is it to ensure that these two use cases are correct in 1.5?
Let's discuss about this and I am more than willing to spend time on improving 1.5. "just enough".

@liggitt
Copy link
Member

liggitt commented Feb 1, 2017

#40666 (and #40260, and other supporting work) would be required to be backported, which is not feasible.

The "create when missing" case does not exercise most of the apply command (the patch computation portion), but create could be used instead.

Use of the apply command to update ThirdPartyResources may appear to be working in 1.4 or 1.5, but is actually sending incorrect data to the server (and doesn't actually update the object in some cases, as noted in #29542). I don't think restoring that behavior is worthwhile.

Until third party resources are stable and supported by kubectl (see kubernetes/enhancements#95), I would recommend using create and/or replace to persist them with kubectl

@ahakanbaba
Copy link
Contributor Author

ahakanbaba commented Feb 1, 2017

What is the state of 1.6 ?
Are the #40666 #40260 and others going to 1.6 ?

@liggitt
Copy link
Member

liggitt commented Feb 1, 2017

Are the #40666 #40260 and others going to 1.6 ?

yes

@pwittrock pwittrock self-assigned this Feb 15, 2017
@pwittrock
Copy link
Member

cc @pwittrock

@liggitt
Copy link
Member

liggitt commented Feb 15, 2017

edit is fixed in #41304 in 1.6

@liggitt
Copy link
Member

liggitt commented Feb 15, 2017

#40545 is working as designed (though see #18023 as a possible alternative)

#29972 Is the only outstanding issue

@ahakanbaba
Copy link
Contributor Author

Maybe once I finish tests in #40841 I could take a look to #29972
For fun ;)

@huggsboson
Copy link
Contributor

This issue is blocking us upgrading to 1.5: #42207

@huggsboson
Copy link
Contributor

We are being affected by this on 1.4 #37278

@mengqiy
Copy link
Member

mengqiy commented Mar 14, 2017

The fix for issue #37278 is PR #37328. It is in 1.5 branch, and has not been backported to 1.4 branch.
cc: @pwittrock

@huggsboson
Copy link
Contributor

huggsboson commented Mar 15, 2017

We've had need of multiple liveness probes to work around some nasty internal stuff we need to do:
#37218

@huggsboson
Copy link
Contributor

#29972 is affecting us.

@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017
@spiffxp
Copy link
Member

spiffxp commented Jun 26, 2017

I'm not sure this issue belongs in kubernetes/kubernetes if it's tracking issues affecting a particular user. A user does not a SIG make.

Does Box have their own issues repo they could use to track these issues instead?

/assign
I'm closing this, please reopen if there's a specific SIG this can be tracked under

@spiffxp
Copy link
Member

spiffxp commented Jun 26, 2017

/close

@ghodss
Copy link
Contributor

ghodss commented Jun 26, 2017

We were advised by @pwittrock to open this issue to help the kube team prioritize issues affecting our deployment. We are happy to stop doing so or to do it in another medium if that is more helpful. We don't really have an issue tracker ourselves that would be relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

10 participants