-
Notifications
You must be signed in to change notification settings - Fork 72
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 handling of enable-console option #412
Conversation
f90d06c
to
da654e2
Compare
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.
Unit tests are passing using both mock and real cluster.
I have also verified --enable-console=true
and --enable-console=false
using the skupper command and it is working properly as well.
One comment or suggestion would be to allow modifying --enable-console
value through SiteConfigUpdate
so users can enable the console on an already initialized namespace, if needed. If you think it is a good idea, I can raise an issue.
I agree that would be a useful enhancement. (The intention is eventually that all aspects can be updated) |
And I believe this is going to be addressed by an existing issue #58. Thank you! |
iiuc, intent is the console should be enabled by default, should default be changed from false to true in cmd/skupper/skupper.go? Otherwise, user will need to '--enable-console on init' |
Yes! Had not realised that it was off by default there. Previously the flag was not properly honoured so would always be on regardless. With this fix though we do need to change the default which I have now done. Well spotted! |
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.
lgtm
built and tried true/false on OpenShift and minikube.
No description provided.