Revert "Fix editing local rke1 and eks clusters (#12583) (#12613)" (#…

…12615) This reverts commit afd037a.
rancher · Nov 14, 2024 · 89605e9 · 89605e9
1 parent afd037a
commit 89605e9
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 80 deletions.
diff --git a/shell/assets/translations/en-us.yaml b/shell/assets/translations/en-us.yaml
@@ -1954,7 +1954,7 @@ cluster:
     pinganyunecs: Pinganyun ECS
     pnap: phoenixNAP
     rackspace: RackSpace
-    rancherkubernetesengine: RKE1
+    rancherkubernetesengine: RKE
     rke2: RKE2
     rke: RKE1
     rkeWindows: Windows (RKE1 only)

diff --git a/shell/components/formatter/ClusterProvider.vue b/shell/components/formatter/ClusterProvider.vue
@@ -25,12 +25,12 @@ export default {
         {{ row.machineProviderDisplay }}
       </span>
     </template>
-    <template v-else-if="row.isImported">
-      {{ t('cluster.provider.imported') }}
-    </template>
     <template v-else-if="row.isCustom">
       {{ t('cluster.provider.custom') }}
     </template>
+    <template v-else-if="row.isImported">
+      {{ t('cluster.provider.imported') }}
+    </template>
     <div class="text-muted">
       {{ row.provisionerDisplay }}
     </div>

diff --git a/shell/models/__tests__/management.cattle.io.cluster.test.ts b/shell/models/__tests__/management.cattle.io.cluster.test.ts
@@ -4,34 +4,12 @@ jest.mock('@shell/utils/clipboard', () => {
   return { copyTextToClipboard: jest.fn(() => Promise.resolve({})) };
 });
 
-const importedRKE2ClusterInfo = { status: { driver: 'rke2', provider: 'rke2' } };
-
-const provisionedRKE2ClusterInfo = { status: { driver: 'rke2', provider: 'imported' } };
-
-const importedK3sClusterInfo = { status: { driver: 'k3s', provider: 'k3s' } };
-
-const provisionedK3sClusterInfo = { status: { driver: 'k3s', provider: 'imported' } };
-
-const importedAksClusterInfo = { spec: { aksConfig: { imported: true } }, status: { provider: 'aks', driver: 'AKS' } };
-
-const provisionedAksClusterInfo = { spec: { aksConfig: { imported: false } }, status: { provider: 'aks', driver: 'AKS' } };
-
-const importedRKE1ClusterInfo = { status: { provider: 'rke', driver: 'imported' } };
-
-const provisionedRKE1ClusterInfo = { status: { provider: 'rke', driver: 'rancherKubernetesEngine' } };
-
-const localRKE1ClusterInfo = { status: { provider: 'rke', driver: 'imported' } };
-
-const localRKE2ClusterInfo = { status: { provider: 'rke2', driver: 'rke2' } };
-
-const localEKSClusterInfo = { status: { provider: 'eks', driver: 'imported' } };
-
 describe('class MgmtCluster', () => {
   describe('provisioner', () => {
     const testCases = [
-      [{ provider: 'rke', driver: 'imported' }, 'imported'],
-      [{ provider: 'k3s', driver: 'K3S' }, 'K3S'],
-      [{ provider: 'aks', driver: 'AKS' }, 'AKS'],
+      [{ provider: 'rke', driver: 'imported' }, 'rke'],
+      [{ provider: 'k3s', driver: 'K3S' }, 'k3s'],
+      [{ provider: 'aks', driver: 'AKS' }, 'aks'],
       [{}, 'imported'],
     ];
 
@@ -42,24 +20,4 @@ describe('class MgmtCluster', () => {
     }
     );
   });
-
-  describe('isImported', () => {
-    it.each([
-      [importedRKE2ClusterInfo, true],
-      [provisionedRKE2ClusterInfo, false],
-      [importedK3sClusterInfo, true],
-      [provisionedK3sClusterInfo, false],
-      [importedAksClusterInfo, true],
-      [provisionedAksClusterInfo, false],
-      [importedRKE1ClusterInfo, true],
-      [provisionedRKE1ClusterInfo, false],
-      [localRKE1ClusterInfo, true],
-      [localRKE2ClusterInfo, true],
-      [localEKSClusterInfo, true]
-    ])('should return isImported based on props data', (clusterData, expected) => {
-      const cluster = new MgmtCluster(clusterData);
-
-      expect(cluster.isImported).toBe(expected);
-    });
-  });
 });
diff --git a/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts b/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
@@ -1,6 +1,31 @@
 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: {} }
+  };
+
   const gkeClusterWithPrivateEndpoint = {
     clusterName: 'test',
     provisioner: 'GKE',
@@ -51,6 +76,67 @@ describe('class ProvCluster', () => {
     });
   });
 
+  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();
+    });
+  });
+
   describe('hasError', () => {
     const conditionsWithoutError = [
       {

diff --git a/shell/models/management.cattle.io.cluster.js b/shell/models/management.cattle.io.cluster.js
@@ -89,31 +89,11 @@ export default class MgmtCluster extends SteveModel {
     return pools.filter((x) => x.spec?.clusterName === this.id);
   }
 
-  get isImported() {
-    if (this.isLocal) {
-      return false;
-    }
-    // imported rke2 and k3s have status.driver === rke2 and k3s respectively
-    // Provisioned rke2 and k3s have status.driver === imported
-    if (this.status?.provider === 'k3s' || this.status?.provider === 'rke2') {
-      return this.status?.driver === this.status?.provider;
-    }
-
-    // imported KEv2
-    const kontainerConfigs = ['aksConfig', 'eksConfig', 'gkeConfig'];
-
-    const isImportedKontainer = kontainerConfigs.filter((key) => {
-      return this.spec?.[key]?.imported === true;
-    }).length;
-
-    if (isImportedKontainer) {
-      return true;
+  get provisioner() {
+    if (this.status?.provider ) {
+      return this.status.provider;
     }
 
-    return this.provisioner === 'imported';
-  }
-
-  get provisioner() {
     // For imported K3s clusters, this.status.driver is 'k3s.'
     return this.status?.driver ? this.status.driver : 'imported';
   }
@@ -137,11 +117,10 @@ export default class MgmtCluster extends SteveModel {
   get providerForEmberParam() {
     // Ember wants one word called provider to tell what component to show, but has much indirect mapping to figure out what it is.
     let provider;
-
-    //  provisioner is status.driver
+    // Provisioner is the "<something>Config" in the model
     const provisioner = KONTAINER_TO_DRIVER[(this.provisioner || '').toLowerCase()] || this.provisioner;
 
-    if ( provisioner === 'rancherKubernetesEngine') {
+    if ( provisioner === 'rancherKubernetesEngine' ) {
       // Look for a cloud provider in one of the node templates
       if ( this.machinePools?.[0] ) {
         provider = this.machinePools[0]?.nodeTemplate?.spec?.driver || null;

diff --git a/shell/models/provisioning.cattle.io.cluster.js b/shell/models/provisioning.cattle.io.cluster.js
@@ -282,7 +282,19 @@ export default class ProvCluster extends SteveModel {
   }
 
   get isImported() {
-    return this.mgmt?.isImported;
+    // As of Rancher v2.6.7, this returns false for imported K3s clusters,
+    // in which this.provisioner is `k3s`.
+
+    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() {
@@ -318,8 +330,7 @@ export default class ProvCluster extends SteveModel {
   }
 
   get isRke1() {
-    // rancherKubernetesEngineConfig is not defined on imported RKE1 clusters
-    return !!this.mgmt?.spec?.rancherKubernetesEngineConfig || this.mgmt?.labels['provider.cattle.io'] === 'rke';
+    return !!this.mgmt?.spec?.rancherKubernetesEngineConfig || this.labels['provider.cattle.io'] === 'rke';
   }
 
   get isHarvester() {
@@ -396,8 +407,6 @@ export default class ProvCluster extends SteveModel {
       provisioner = 'k3s';
     } else if ( this.isImportedRke2 ) {
       provisioner = 'rke2';
-    } else if ((this.isImported || this.isLocal) && this.isRke1) {
-      provisioner = 'rke';
     }
 
     return this.$rootGetters['i18n/withFallback'](`cluster.provider."${ provisioner }"`, null, ucFirst(provisioner));