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: Fix starting multiple che instances in the same cluster #598

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/api/kube.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,14 @@ export class KubeHelper {
}
}

async createClusterRoleFromFile(filePath: string) {
async createClusterRoleFromFile(filePath: string, roleName?: string) {
const yamlRole = this.safeLoadFromYamlFile(filePath) as V1ClusterRole
if (!yamlRole.metadata || !yamlRole.metadata.name) {
throw new Error(`Cluster Role read from ${filePath} must have name specified`)
}
if (roleName) {
yamlRole.metadata!.name = roleName
}
const k8sRbacAuthApi = this.kc.makeApiClient(RbacAuthorizationV1Api)
try {
const res = await k8sRbacAuthApi.createClusterRole(yamlRole)
Expand All @@ -270,12 +276,15 @@ export class KubeHelper {
}
}

async replaceClusterRoleFromFile(filePath: string) {
async replaceClusterRoleFromFile(filePath: string, roleName?: string) {
const yamlRole = this.safeLoadFromYamlFile(filePath) as V1ClusterRole
const k8sRbacAuthApi = this.kc.makeApiClient(RbacAuthorizationV1Api)
if (!yamlRole.metadata || !yamlRole.metadata.name) {
throw new Error(`Cluster Role read from ${filePath} must have name specified`)
}
if (roleName) {
yamlRole.metadata!.name = roleName
}
try {
const res = await k8sRbacAuthApi.replaceClusterRole(yamlRole.metadata.name, yamlRole)
return res.response.statusCode
Expand Down
55 changes: 26 additions & 29 deletions src/tasks/installers/operator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export class OperatorTasks {
* Returns tasks list which perform preflight platform checks.
*/
startTasks(flags: any, command: Command): Listr {
const clusterRoleName = `${this.operatorClusterRole}-${flags.chenamespace}`
const clusterRoleBindingName = `${this.operatorClusterRoleBinding}-${flags.chenamespace}`
const che = new CheHelper(flags)
const kube = new KubeHelper(flags)
return new Listr([
Expand Down Expand Up @@ -90,14 +92,14 @@ export class OperatorTasks {
}
},
{
title: `Create ClusterRole ${this.operatorClusterRole}`,
title: `Create ClusterRole ${clusterRoleName}`,
task: async (_ctx: any, task: any) => {
const exist = await kube.clusterRoleExist(this.operatorClusterRole)
const exist = await kube.clusterRoleExist(clusterRoleName)
if (exist) {
task.title = `${task.title}...It already exists.`
} else {
const yamlFilePath = this.resourcesPath + 'cluster_role.yaml'
const statusCode = await kube.createClusterRoleFromFile(yamlFilePath)
const statusCode = await kube.createClusterRoleFromFile(yamlFilePath, clusterRoleName)
if (statusCode === 403) {
command.error('ERROR: It looks like you don\'t have enough privileges. You need to grant more privileges to current user or use a different user. If you are using minishift you can "oc login -u system:admin"')
}
Expand All @@ -119,13 +121,13 @@ export class OperatorTasks {
}
},
{
title: `Create ClusterRoleBinding ${this.operatorClusterRoleBinding}`,
title: `Create ClusterRoleBinding ${clusterRoleBindingName}`,
task: async (_ctx: any, task: any) => {
const exist = await kube.clusterRoleBindingExist(this.operatorRoleBinding)
const exist = await kube.clusterRoleBindingExist(clusterRoleBindingName)
if (exist) {
task.title = `${task.title}...It already exists.`
} else {
await kube.createClusterRoleBinding(this.operatorClusterRoleBinding, this.operatorServiceAccount, flags.chenamespace, this.operatorClusterRole)
await kube.createClusterRoleBinding(clusterRoleBindingName, this.operatorServiceAccount, flags.chenamespace, clusterRoleName)
task.title = `${task.title}...done.`
}
}
Expand Down Expand Up @@ -220,6 +222,8 @@ export class OperatorTasks {
}

updateTasks(flags: any, command: Command): Listr {
const clusterRoleName = `${this.operatorClusterRole}-${flags.chenamespace}`
const clusterRoleBindingName = `${this.operatorClusterRoleBinding}-${flags.chenamespace}`
const kube = new KubeHelper(flags)
return new Listr([
{
Expand Down Expand Up @@ -264,18 +268,18 @@ export class OperatorTasks {
}
},
{
title: `Updating ClusterRole ${this.operatorClusterRole}`,
title: `Updating ClusterRole ${clusterRoleName}`,
task: async (_ctx: any, task: any) => {
const exist = await kube.clusterRoleExist(this.operatorClusterRole)
const exist = await kube.clusterRoleExist(clusterRoleName)
const yamlFilePath = this.resourcesPath + 'cluster_role.yaml'
if (exist) {
const statusCode = await kube.replaceClusterRoleFromFile(yamlFilePath)
const statusCode = await kube.replaceClusterRoleFromFile(yamlFilePath, clusterRoleName)
if (statusCode === 403) {
command.error('ERROR: It looks like you don\'t have enough privileges. You need to grant more privileges to current user or use a different user. If you are using minishift you can "oc login -u system:admin"')
}
task.title = `${task.title}...updated.`
} else {
const statusCode = await kube.createClusterRoleFromFile(yamlFilePath)
const statusCode = await kube.createClusterRoleFromFile(yamlFilePath, clusterRoleName)
if (statusCode === 403) {
command.error('ERROR: It looks like you don\'t have enough privileges. You need to grant more privileges to current user or use a different user. If you are using minishift you can "oc login -u system:admin"')
}
Expand All @@ -298,14 +302,14 @@ export class OperatorTasks {
}
},
{
title: `Updating ClusterRoleBinding ${this.operatorClusterRoleBinding}`,
title: `Updating ClusterRoleBinding ${clusterRoleBindingName}`,
task: async (_ctx: any, task: any) => {
const exist = await kube.clusterRoleBindingExist(this.operatorRoleBinding)
const exist = await kube.clusterRoleBindingExist(clusterRoleBindingName)
if (exist) {
await kube.replaceClusterRoleBinding(this.operatorClusterRoleBinding, this.operatorServiceAccount, flags.chenamespace, this.operatorClusterRole)
await kube.replaceClusterRoleBinding(clusterRoleBindingName, this.operatorServiceAccount, flags.chenamespace, clusterRoleName)
task.title = `${task.title}...updated.`
} else {
await kube.createClusterRoleBinding(this.operatorClusterRoleBinding, this.operatorServiceAccount, flags.chenamespace, this.operatorClusterRole)
await kube.createClusterRoleBinding(clusterRoleBindingName, this.operatorServiceAccount, flags.chenamespace, clusterRoleName)
task.title = `${task.title}...created new one.`
}
}
Expand Down Expand Up @@ -363,6 +367,8 @@ export class OperatorTasks {
*/
deleteTasks(flags: any): ReadonlyArray<Listr.ListrTask> {
let kh = new KubeHelper(flags)
const clusterRoleName = `${this.operatorClusterRole}-${flags.chenamespace}`
const clusterRoleBindingName = `${this.operatorClusterRoleBinding}-${flags.chenamespace}`
return [{
title: `Delete the CR ${this.operatorCheCluster} of type ${this.cheClusterCrd}`,
task: async (_ctx: any, task: any) => {
Expand All @@ -376,15 +382,6 @@ export class OperatorTasks {
}
}
},
{
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @l0rd

Copy link
Collaborator

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

Copy link
Collaborator

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

title: `Delete CRD ${this.cheClusterCrd}`,
task: async (_ctx: any, task: any) => {
if (await kh.crdExist(this.cheClusterCrd)) {
await kh.deleteCrd(this.cheClusterCrd)
}
task.title = await `${task.title}...OK`
}
},
{
title: `Delete role binding ${this.operatorRoleBinding}`,
task: async (_ctx: any, task: any) => {
Expand All @@ -404,19 +401,19 @@ export class OperatorTasks {
}
},
{
title: `Delete cluster role binding ${this.operatorClusterRoleBinding}`,
title: `Delete cluster role binding ${clusterRoleBindingName}`,
task: async (_ctx: any, task: any) => {
if (await kh.clusterRoleBindingExist(this.operatorClusterRoleBinding)) {
await kh.deleteClusterRoleBinding(this.operatorClusterRoleBinding)
if (await kh.clusterRoleBindingExist(clusterRoleBindingName)) {
await kh.deleteClusterRoleBinding(clusterRoleBindingName)
}
task.title = await `${task.title}...OK`
}
},
{
title: `Delete cluster role ${this.operatorClusterRole}`,
title: `Delete cluster role ${clusterRoleName}`,
task: async (_ctx: any, task: any) => {
if (await kh.clusterRoleExist(this.operatorClusterRole)) {
await kh.deleteClusterRole(this.operatorClusterRole)
if (await kh.clusterRoleExist(clusterRoleName)) {
await kh.deleteClusterRole(clusterRoleName)
}
task.title = await `${task.title}...OK`
}
Expand Down