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

Improve linkerd upgrade | kubectl apply output format #2956

Closed
klingerf opened this issue Jun 18, 2019 · 10 comments · Fixed by #4053
Closed

Improve linkerd upgrade | kubectl apply output format #2956

klingerf opened this issue Jun 18, 2019 · 10 comments · Fixed by #4053

Comments

@klingerf
Copy link
Member

I just ran linkerd upgrade | kubectl apply -f - on one of my AKS clusters, to upgrade the control plane from edge-19.5.3 to edge-19.6.2. It printed:

$ linkerd upgrade | kubectl apply -f -
namespace/linkerd configured
clusterrole.rbac.authorization.k8s.io/linkerd-linkerd-identity unchanged
clusterrolebinding.rbac.authorization.k8s.io/linkerd-linkerd-identity unchanged
serviceaccount/linkerd-identity unchanged
clusterrole.rbac.authorization.k8s.io/linkerd-linkerd-controller unchanged
clusterrolebinding.rbac.authorization.k8s.io/linkerd-linkerd-controller unchanged
serviceaccount/linkerd-controller unchanged
serviceaccount/linkerd-web unchanged
customresourcedefinition.apiextensions.k8s.io/serviceprofiles.linkerd.io configured
clusterrole.rbac.authorization.k8s.io/linkerd-linkerd-prometheus unchanged
clusterrolebinding.rbac.authorization.k8s.io/linkerd-linkerd-prometheus unchanged
serviceaccount/linkerd-prometheus unchanged
serviceaccount/linkerd-grafana unchanged
clusterrole.rbac.authorization.k8s.io/linkerd-linkerd-proxy-injector configured
clusterrolebinding.rbac.authorization.k8s.io/linkerd-linkerd-proxy-injector configured
serviceaccount/linkerd-proxy-injector unchanged
secret/linkerd-proxy-injector-tls created

√ You're on your way to upgrading Linkerd!
Visit this URL for further instructions: https://linkerd.io/upgrade/#nextsteps

Warning: kubectl apply should be used on resource created by either kubectl create --save-config or kubectl apply
mutatingwebhookconfiguration.admissionregistration.k8s.io/linkerd-proxy-injector-webhook-config configured
clusterrole.rbac.authorization.k8s.io/linkerd-linkerd-sp-validator configured
clusterrolebinding.rbac.authorization.k8s.io/linkerd-linkerd-sp-validator configured
...

It's a bit jarring that the "You're on your way to upgrading" message gets printed in the middle of the kubectl apply output. I think the issue is that linkerd upgrade writes injected objects incrementally to stdout, and kubectl apply starts applying them before the upgrade command has finished running. I can't find it offhand, but we've run into this same issue before, and I think we fixed it by writing the config to a buffer, and then writing the contents of the buffer to stdout once all objects are injected.

@grampelberg
Copy link
Contributor

@klingerf should we be concerned about the warning as well? It sounds scary even though I don't believe it impacts users at all.

@grampelberg grampelberg added this to To do in 2.5 - Release via automation Jun 18, 2019
@ihcsim
Copy link
Contributor

ihcsim commented Jun 18, 2019

The warnings should be benign afaict. We get that one-off warning on the linkerd-proxy-injector-webhook-config mwc, because previously, it was created at runtime by the proxy injector, not through kubectl. I am not sure about the warnings on the cluster roles and cluster role bindings though. Did @klingerf manually (re-)create them somehow prior to the upgrade?

@klingerf
Copy link
Member Author

@grampelberg Ah, I totally missed that warning about the MWC in the kubectl output. @ihcsim's explanation for what's going on there makes sense.

FWIW, I'm not sure that "You're on your way to upgrading Linkerd" message is all that useful. We don't print an equivalent when running linkerd install. And it links to an anchor (#nextsteps) that no longer exists, as far as I can tell. What we if we just removed that text?

@grampelberg
Copy link
Contributor

👍 from me.

@grampelberg grampelberg removed the priority/P0 Release Blocker label Jul 15, 2019
@grampelberg grampelberg added this to To do in Help Wanted via automation Jul 15, 2019
@grampelberg grampelberg removed this from To do in 2.5 - Release Aug 5, 2019
@wez470
Copy link

wez470 commented Oct 5, 2019

I'm looking for a first issue to do. Would it be okay for me to pick this up?

@wmorgan
Copy link
Member

wmorgan commented Oct 6, 2019

@wez470 go for it. Since this is a few months old I would double-check that the issue still exists, first. And if you come up with a solution, I would propose it in this issue before jumping straight to a PR. There are several competing concerns when it comes to upgrade flow.

@grampelberg
Copy link
Contributor

Definitely still a problem, we'd love your help @wez470 !

@wez470
Copy link

wez470 commented Dec 2, 2019

I apologize, but I may not get to doing this issue. I still might, but wanted to say that anyone who sees this should feel free to do it.

@supra08
Copy link
Contributor

supra08 commented Feb 14, 2020

Hi @klingerf! I would like to work on this issue.

supra08 added a commit to supra08/linkerd2 that referenced this issue Feb 14, 2020
okMessage is displayed in the middle while linkerd upgrade

Remove the okMessage since it was inconsistent with the linkerd install output

run linkerd upgrade | kubectl apply -f - to see anymore such message

Fixes linkerd#2956

Signed-off-by: Supratik Das <rick.das08@gmail.com>
@grampelberg
Copy link
Contributor

@supra08 go for it! Looks like you got a PR up and everything =)

supra08 added a commit to supra08/linkerd2 that referenced this issue Feb 18, 2020
okMessage is displayed in the middle while linkerd upgrade

Remove the okMessage since it was inconsistent with the linkerd install output

run linkerd upgrade | kubectl apply -f - to see anymore such message

Fixes linkerd#2956

Signed-off-by: Supratik Das <rick.das08@gmail.com>
Help Wanted automation moved this from To do to Done Feb 20, 2020
alpeb pushed a commit that referenced this issue Feb 20, 2020
* Improve kubectl apply format by removing misplaced message

Fixes #2956

Also separate stderr messages with a new line

Signed-off-by: Supratik Das <rick.das08@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Help Wanted
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants