-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat: Add domain checks for Kubernetes family infrastructures #615
Conversation
120238f
to
90380ee
Compare
export function getPingClusterTask(flags: any): Listr.ListrTask { | ||
return { | ||
title: 'Check if cluster accessible', | ||
skip: () => flags['skip-cluster-check'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|| !flags.domain
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because I want to show warning
} | ||
|
||
if (!await checkServer(domain, 80) || !await checkServer(domain, 443)) { | ||
throw new Error(`Cannot reach cluster "${domain}". To skip this check add "--skip-cluster-check" flag.`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neither ${domain}:80 nor ${domain}:443
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of
Cannot reach cluster "${domain}". To skip this check add "--skip-cluster-check" flag.
it is better to log
Cannot reach neither '${domain}:80' nor '${domain}:443'. It means the incorrect domain is used. Please, set the correct domain or bypass the check by using '--skip-domain-check' flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it is a good idea to print domain twice. I would like to avoid it.
90380ee
to
5a77845
Compare
src/commands/server/start.ts
Outdated
@@ -140,6 +140,10 @@ export default class Start extends Command { | |||
'skip-version-check': flags.boolean({ | |||
description: 'Skip minimal versions check.', | |||
default: false | |||
}), | |||
'skip-cluster-check': flags.boolean({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip-domain-check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--skip-cluster-availability-check
as we agreed
} | ||
|
||
if (!await checkServer(domain, 80) || !await checkServer(domain, 443)) { | ||
throw new Error(`Cannot reach cluster "${domain}". To skip this check add "--skip-cluster-check" flag.`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of
Cannot reach cluster "${domain}". To skip this check add "--skip-cluster-check" flag.
it is better to log
Cannot reach neither '${domain}:80' nor '${domain}:443'. It means the incorrect domain is used. Please, set the correct domain or bypass the check by using '--skip-domain-check' flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls, consider my comments
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
5a77845
to
fcffd86
Compare
Signed-off-by: Mykola Morhun mmorhun@redhat.com
What does this PR do?
Adds check whether Kubernetes cluster is reachable when installing Che.
The check could be disabled by
--skip-cluster-check
flag.What issues does this PR fix or reference?
#613