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

fix: do not fetch service account if it's unneeded #340

Closed

Conversation

sleshchenko
Copy link
Contributor

@sleshchenko sleshchenko commented Oct 4, 2019

What does this PR do?

This PR is made as a result of the following discussion #257 (comment)

Do not fetch the service account if it's unneeded. See attachments:
before:
Screenshot_20191004_152445

After:
minishift-addon
Screenshot_20191004_152359
operator
Screenshot_20191004_152412

It also improves error message when the current context is not set:
before
Screenshot_20191004_153539
now
Screenshot_20191004_153524

I did not check it on an installation where healthz endpoint requires token since I do not know when to find it or how to set up, like even secure OSIO installation have healthz as public https://console.starter-us-east-2.openshift.com/healthz

It's like an unplanned quick fix, and some things can be done not in the best way. So, I'm open to proposals related to refactoring or messages.

What issues does this PR fix or reference?

Fixes eclipse-che/che#14534

It partially fixes(there are still may be places that should be addressed) eclipse-che/che#14665

How to test it?

Here are built chectl(based on the master 25.10.2019) that can be used for testing:
https://dropmefiles.com/lVXSh <- Click more to select only package specific to your OS

@benoitf
Copy link
Collaborator

benoitf commented Oct 4, 2019

Amazon EC2 has secured healthz

@sleshchenko
Copy link
Contributor Author

@benoitf yeah, thanks. It's in your issue but I missed this.
Do you happen to have EC2 K8s running to check my changes?

@benoitf
Copy link
Collaborator

benoitf commented Oct 4, 2019

@sleshchenko no sorry my trial account has expired :-/

@sleshchenko
Copy link
Contributor Author

@richardmward I see your issue where you use EC2 K8s for deploying of Che.
Could you please help me to test my PR?
It could be enough if you:

  1. Download archive for your OS https://dropmefiles.com/lVXSh
  2. Unpack it
  3. rm -rf $HOME/.local/share/chectl/client to avoid cache conflicts with your installed version.
  4. From unpacked folder execute ./bin/chectl server:stop --chenamespace=non-existing
    if you get
serg@localhost:~/projects/chectl$ chectl server:stop --chenamespace non-existing
  ✔ Verify Kubernetes API...OK (it's OpenShift)
  ✔ Verify if Che is deployed into namespace "non-existing"...it is not
  ✖ Deployment doesn't exist

If you get another error - probably something is wrong in my fix and let me know.

After testing you're able just remove downloaded files and also do rm $HOME/.local/share/chectl/client again. After that you're able to use your installed chectl version again.

Thanks in advance!!!

Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
@tolusha
Copy link
Collaborator

tolusha commented Feb 20, 2020

Closed in favor of
#524

@tolusha tolusha closed this Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chectl: k8s health check fails if logged in user does not have cluster admin rights
3 participants