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

Fixed RKE cluster being shown as Imported #9868

Merged
merged 3 commits into from
Nov 1, 2023
Merged
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
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
Loading