Skip to content

Commit

Permalink
Fixed RKE cluster being shown as Imported (#9868)
Browse files Browse the repository at this point in the history
  • Loading branch information
eva-vashkevich authored Nov 1, 2023
1 parent 92d8103 commit 6d4f75a
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 50 deletions.
19 changes: 1 addition & 18 deletions shell/components/formatter/ClusterProvider.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,6 @@ export default {
required: true
},
},
data(props) {
const mgmt = props.row?.mgmt;
return {
// The isImported getter on the provisioning cluster
// model doesn't work for imported K3s clusters, in
// which case it returns 'k3s' instead of 'imported.'
// This is the workaround.
isImported: mgmt?.providerForEmberParam === 'import' ||
// when imported cluster is GKE
!!mgmt?.spec?.gkeConfig?.imported ||
// or AKS
!!mgmt?.spec?.aksConfig?.imported ||
// or EKS
!!mgmt?.spec?.eksConfig?.imported
};
},
};
</script>

Expand All @@ -45,7 +28,7 @@ export default {
<template v-else-if="row.isCustom">
{{ t('cluster.provider.custom') }}
</template>
<template v-else-if="isImported">
<template v-else-if="row.isImported">
{{ t('cluster.provider.imported') }}
</template>
<div class="text-muted">
Expand Down
28 changes: 0 additions & 28 deletions shell/components/formatter/__tests__/ClusterProvider.test.ts

This file was deleted.

19 changes: 19 additions & 0 deletions shell/models/__tests__/management.cattle.io.cluster.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import MgmtCluster from '@shell/models/management.cattle.io.cluster';

describe('class MgmtCluster', () => {
describe('provisioner', () => {
const testCases = [
[{ provider: 'rke', driver: 'imported' }, 'rke'],
[{ provider: 'k3s', driver: 'K3S' }, 'k3s'],
[{ provider: 'aks', driver: 'AKS' }, 'aks'],
[{}, 'imported'],
];

it.each(testCases)('should return provisioner value properly based on the props data', (clusterData: Object, expected: String) => {
const cluster = new MgmtCluster({ status: clusterData });

expect(cluster.provisioner).toBe(expected);
}
);
});
});
90 changes: 90 additions & 0 deletions shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import ProvCluster from '@shell/models/provisioning.cattle.io.cluster';

describe('class ProvCluster', () => {
const importedClusterInfo = {
clusterName: 'test', provisioner: 'imported', mgmt: { spec: { gkeConfig: {} } }, spec: {}
};
const importedGkeClusterInfo = {
clusterName: 'test', provisioner: 'rke2', mgmt: { spec: { gkeConfig: { imported: true } } }
};
const importedAksClusterInfo = {
clusterName: 'test', provisioner: 'rke2', mgmt: { spec: { aksConfig: { imported: true } } }
};
const importedEksClusterInfo = {
clusterName: 'test', provisioner: 'rke2', mgmt: { spec: { eksConfig: { imported: true } } }
};
const notImportedGkeClusterInfo = {
clusterName: 'test', provisioner: 'rke2', mgmt: { spec: { gkeConfig: { imported: false } }, rkeConfig: {} }
};
const importedClusterInfoWithProviderForEmberParam = {
clusterName: 'test', provisioner: 'rke2', mgmt: { providerForEmberParam: 'import' }
};
const localClusterInfo = {
clusterName: 'test', provisioner: 'imported', mgmt: { isLocal: true, spec: { gkeConfig: {} } }, spec: {}
};
const doRke2Info = {
clusterName: 'test', provisioner: 'rke2', mgmt: { isLocal: false, providerForEmberParam: 'import' }, spec: { rkeConfig: {} }
};

describe('isImported', () => {
const testCases = [
[importedClusterInfo, true],
[importedGkeClusterInfo, true],
[importedAksClusterInfo, true],
[importedEksClusterInfo, true],
[notImportedGkeClusterInfo, false],
[importedClusterInfoWithProviderForEmberParam, true],
[localClusterInfo, false],
[doRke2Info, false],
[{}, false],
];
const resetMocks = () => {
// Clear all mock function calls:
jest.clearAllMocks();
};

it.each(testCases)('should return the isImported value properly based on the props data', (clusterData: Object, expected: Boolean) => {
const cluster = new ProvCluster({ spec: clusterData.spec });

jest.spyOn(cluster, 'mgmt', 'get').mockReturnValue(
clusterData.mgmt
);
jest.spyOn(cluster, 'provisioner', 'get').mockReturnValue(
clusterData.provisioner
);

expect(cluster.isImported).toBe(expected);
resetMocks();
}
);
});

describe('mgmt', () => {
const testCases = [
[importedClusterInfo, importedClusterInfo.mgmt],
[importedGkeClusterInfo, importedGkeClusterInfo.mgmt],
[importedAksClusterInfo, importedAksClusterInfo.mgmt],
[importedEksClusterInfo, importedEksClusterInfo.mgmt],
[notImportedGkeClusterInfo, notImportedGkeClusterInfo.mgmt],
[importedClusterInfoWithProviderForEmberParam, importedClusterInfoWithProviderForEmberParam.mgmt],
[localClusterInfo, localClusterInfo.mgmt],
[doRke2Info, doRke2Info.mgmt],
[{}, null],
];

const resetMocks = () => {
// Clear all mock function calls:
jest.clearAllMocks();
};

it.each(testCases)('should return the isImported value properly based on the props data', (clusterData: Object, expected: Object) => {
const clusterMock = jest.fn(() => clusterData.mgmt);
const ctx = { rootGetters: { 'management/byId': clusterMock } };
const cluster = new ProvCluster({ status: { clusterName: clusterData.clusterName } }, ctx);

expect(cluster.mgmt).toBe(expected);
resetMocks();
}
);
});
});
4 changes: 4 additions & 0 deletions shell/models/management.cattle.io.cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ export default class MgmtCluster extends HybridModel {
}

get provisioner() {
if (this.status?.provider ) {
return this.status.provider;
}

// For imported K3s clusters, this.status.driver is 'k3s.'
return this.status?.driver ? this.status.driver : 'imported';
}
Expand Down
20 changes: 16 additions & 4 deletions shell/models/provisioning.cattle.io.cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,24 @@ export default class ProvCluster extends SteveModel {
return providers.includes(this.provisioner);
}

get isLocal() {
return this.mgmt?.isLocal;
}

get isImported() {
// As of Rancher v2.6.7, this returns false for imported K3s clusters,
// in which this.provisioner is `k3s`.
return this.provisioner === 'imported';

const isImportedProvisioner = this.provisioner === 'imported';
const isImportedSpecialCases = this.mgmt?.providerForEmberParam === 'import' ||
// when imported cluster is GKE
!!this.mgmt?.spec?.gkeConfig?.imported ||
// or AKS
!!this.mgmt?.spec?.aksConfig?.imported ||
// or EKS
!!this.mgmt?.spec?.eksConfig?.imported;

return !this.isLocal && (isImportedProvisioner || (!this.isRke2 && !this.mgmt?.machineProvider && isImportedSpecialCases));
}

get isCustom() {
Expand All @@ -262,8 +276,6 @@ export default class ProvCluster extends SteveModel {
}

get isImportedK3s() {
// As of Rancher v2.6.7, this returns false for imported K3s clusters,
// in which this.provisioner is `k3s`.
return this.isImported && this.isK3s;
}

Expand All @@ -280,7 +292,7 @@ export default class ProvCluster extends SteveModel {
}

get isRke1() {
return !!this.mgmt?.spec?.rancherKubernetesEngineConfig;
return !!this.mgmt?.spec?.rancherKubernetesEngineConfig || this.labels['provider.cattle.io'] === 'rke';
}

get isHarvester() {
Expand Down

0 comments on commit 6d4f75a

Please sign in to comment.