-
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
fix: Fix starting multiple che instances in the same cluster #598
fix: Fix starting multiple che instances in the same cluster #598
Conversation
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
I still want to try better handle code part with clean up CRD. Currently I removed clean up CRD on remove Che, because CRD has global scope. We can not create CRD in namespace scope kubernetes/kubernetes#65551 per Che installation. When we remove CRD to clean one Che installation we actually remove CRD for all Che instances and breaks all installation in the same cluster... |
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.
Only one question about CRD. The rest is nice.
@@ -376,15 +382,6 @@ export class OperatorTasks { | |||
} | |||
} | |||
}, | |||
{ |
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 understand that we should not delete the CRD when it might still be used by other installations.
OTOH for now, on the chectl server:start
, the CRD is only created if it didn't exist as shown here.
That means that deleting a Che server with a chectl
version and reinstalling with a new version of chectl
will not update the CRD anymore.
That's a hard-to-solve question here, since we can manage only one version of the CRD.
That's a typical case where OLM-based installation has a real value, because the logic of CRD update is managed by OLM itself.
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.
cc @l0rd
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.
@davidfestal
If we are not going to delete CRD file, I would suggest to update it every time we install che using checlt
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.
It allows to keep CRD up to date
What does this PR do?
Provide unique clusterrole and clusterrole binding per che instance in the same cluster. Use namespace suffix for this purpose:
${che-role-name}-namespace
and${che-rolebinding-name}-namespace
What issues does this PR fix or reference?
eclipse-che/che#15345
Mostly for https://issues.redhat.com/browse/CRW-505?focusedCommentId=13816159&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel