Skip to content

Commit

Permalink
do not allow cluster upgrades if node pools would fall more than one …
Browse files Browse the repository at this point in the history
…minor version behind (#11489)
  • Loading branch information
mantis-toboggan-md committed Jul 18, 2024
1 parent ddd2dee commit ba26cf5
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 3 deletions.
39 changes: 37 additions & 2 deletions pkg/eks/components/Config.vue
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
<script lang="ts">
import { defineComponent, PropType } from 'vue';
import { EKSConfig, AWS } from '../types';
import { _EDIT, _VIEW } from '@shell/config/query-params';
import { _EDIT, _VIEW, _CREATE } from '@shell/config/query-params';
import semver from 'semver';
import { Store, mapGetters } from 'vuex';
import { MANAGEMENT } from '@shell/config/types';
import { SETTING } from '@shell/config/settings';
import RadioGroup from '@components/Form/Radio/RadioGroup.vue';
import Banner from '@components/Banner/Banner.vue';
import LabeledSelect from '@shell/components/form/LabeledSelect.vue';
import KeyValue from '@shell/components/form/KeyValue.vue';
Expand All @@ -23,14 +24,16 @@ export default defineComponent({
RadioGroup,
KeyValue,
Checkbox,
LabeledInput
LabeledInput,
Banner
},
props: {
mode: {
type: String,
default: _EDIT
},
isNewOrUnprovisioned: {
type: Boolean,
default: true
Expand All @@ -40,6 +43,7 @@ export default defineComponent({
type: Boolean,
default: false
},
eksRoles: {
type: Array,
default: () => []
Expand All @@ -49,26 +53,32 @@ export default defineComponent({
type: Object,
default: () => {}
},
kmsKey: {
type: String,
default: ''
},
serviceRole: {
type: String,
default: ''
},
kubernetesVersion: {
type: String,
default: ''
},
enableNetworkPolicy: {
type: Boolean,
default: false
},
ebsCSIDriver: {
type: Boolean,
default: false
},
config: {
type: Object as PropType<EKSConfig>,
required: true
Expand Down Expand Up @@ -120,6 +130,7 @@ export default defineComponent({
},
immediate: true
},
'encryptSecrets'(neu) {
if (!neu) {
this.$emit('update:kmsKey', '');
Expand All @@ -146,6 +157,23 @@ export default defineComponent({
computed: {
...mapGetters({ t: 'i18n/t' }),
// the control plane k8s version can't be more than one minor version ahead of any node pools
// verify that all nodepools are on the same version as the control plane before showing upgrade optiopns
canUpgrade(): boolean {
if (this.mode === _CREATE) {
return true;
}
const nodeGroups = this.config?.nodeGroups || [];
const needsUpgrade = nodeGroups.filter((group) => semver.gt(semver.coerce(this.originalVersion), semver.coerce(group.version)) || group._isUpgrading);
return !needsUpgrade.length;
},
hasUpgradesAvailable() {
return this.versionOptions.filter((opt) => !opt.disabled).length > 1;
},
versionOptions(): {value: string, label: string, disabled?:boolean}[] {
return this.allKubernetesVersions.reduce((versions, v: string) => {
const coerced = semver.coerce(v);
Expand Down Expand Up @@ -238,6 +266,12 @@ export default defineComponent({

<template>
<div>
<Banner
v-if="!canUpgrade && hasUpgradesAvailable"
color="info"
label-key="eks.version.upgradeDisallowed"
data-testid="eks-version-upgrade-disallowed-banner"
/>
<div
:style="{'display':'flex',
'align-items':'center'}"
Expand All @@ -253,6 +287,7 @@ export default defineComponent({
:taggable="true"
:searchable="true"
data-testid="eks-version-dropdown"
:disabled="!canUpgrade && hasUpgradesAvailable"
@input="$emit('update:kubernetesVersion', $event)"
/>
</div>
Expand Down
1 change: 1 addition & 0 deletions pkg/eks/components/CruEKS.vue
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ export default defineComponent({
:request-spot-instances.sync="node.requestSpotInstances"
:labels.sync="node.labels"
:version.sync="node.version"
:pool-is-upgrading.sync="node._isUpgrading"
:cluster-version="config.kubernetesVersion"
:original-cluster-version="originalVersion"
:region="config.region"
Expand Down
7 changes: 7 additions & 0 deletions pkg/eks/components/NodeGroup.vue
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ export default defineComponent({
default: null
},
poolIsUpgrading: {
type: Boolean,
default: false
},
loadingInstanceTypes: {
type: Boolean,
default: false
Expand Down Expand Up @@ -408,8 +413,10 @@ export default defineComponent({
set(neu: boolean) {
if (neu) {
this.$emit('update:version', this.clusterVersion);
this.$emit('update:poolIsUpgrading', true);
} else {
this.$emit('update:version', this.originalNodeVersion);
this.$emit('update:poolIsUpgrading', false);
}
}
},
Expand Down
64 changes: 64 additions & 0 deletions pkg/eks/components/__tests__/Config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,68 @@ describe('eKS K8s configuration', () => {

expect(versionOptions.filter((opt: {label: string, value: string, disabled?:boolean}) => !opt.disabled).map((opt: {label: string, value: string, disabled?:boolean}) => opt.value)).toStrictEqual(enabledVersions);
});

it('should not allow the user to upgrade if any node groups are of a lower version', async() => {
const setup = requiredSetup({ value: '>1.24' });
const wrapper = shallowMount(Config, {
propsData: {
config: {
amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.25' }]
},
originalVersion: '1.26',
},
...setup
});

await setCredential(wrapper);

const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');

expect(versionDropdown.props().disabled).toBe(true);
expect(upgradesDisallowedBanner.isVisible()).toBe(true);
});

// setting/unsetting _isUpgrading is tested in ./NodeGroup.test.ts
it('should not allow the user to upgrade if any node groups are currently being upgraded', async() => {
const setup = requiredSetup({ value: '>1.24' });
const wrapper = shallowMount(Config, {
propsData: {
config: {
amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.26', _isUpgrading: true }]
},
originalVersion: '1.26',
},
...setup
});

await setCredential(wrapper);

const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');

expect(versionDropdown.props().disabled).toBe(true);
expect(upgradesDisallowedBanner.isVisible()).toBe(true);
});

it('should allow the user to upgrade if all node groups are the same version as the current cluster version', async() => {
const setup = requiredSetup({ value: '>1.24' });
const wrapper = shallowMount(Config, {
propsData: {
config: {
amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.26' }]
},
originalVersion: '1.26',
},
...setup
});

await setCredential(wrapper);

const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');

expect(versionDropdown.props().disabled).toBe(false);
expect(upgradesDisallowedBanner.exists()).toBe(false);
});
});
57 changes: 57 additions & 0 deletions pkg/eks/components/__tests__/NodeGroup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,63 @@ describe('eks node groups: edit', () => {
expect(wrapper.emitted('update:version')?.[0]?.[0]).toBe('1.24');
});

it('should report that a pool is being upgraded when the upgrade checkbox is checked', async() => {
const setup = requiredSetup();

const wrapper = shallowMount(NodeGroup, {
propsData: {
launchTemplate: {},
region: 'foo',
amazonCredentialSecret: 'bar',
version: '1.23',
clusterVersion: '1.24',
originalClusterVersion: '1.24',
isNewOrUnprovisioned: false
},
...setup
});

expect(wrapper.emitted('update:poolIsUpgrading')).toBeUndefined();

const upgradeVersionCheckbox = wrapper.find('[data-testid="eks-version-upgrade-checkbox"]');

upgradeVersionCheckbox.vm.$emit('input', true);
await wrapper.vm.$nextTick();

expect(wrapper.emitted('update:poolIsUpgrading')?.[0]?.[0]).toBe(true);
});

it('should report that a pool is NOT being upgraded when the upgrade checkbox is unchecked', async() => {
const setup = requiredSetup();

const wrapper = shallowMount(NodeGroup, {
propsData: {
launchTemplate: {},
region: 'foo',
amazonCredentialSecret: 'bar',
version: '1.23',
clusterVersion: '1.24',
originalClusterVersion: '1.24',
isNewOrUnprovisioned: false
},
...setup
});

expect(wrapper.emitted('update:poolIsUpgrading')).toBeUndefined();

const upgradeVersionCheckbox = wrapper.find('[data-testid="eks-version-upgrade-checkbox"]');

wrapper.setProps({ version: '1.24' });
await wrapper.vm.$nextTick();

expect(upgradeVersionCheckbox.props().value).toBe(true);

upgradeVersionCheckbox.vm.$emit('input', false);
await wrapper.vm.$nextTick();

expect(wrapper.emitted('update:poolIsUpgrading')?.[0]?.[0]).toBe(false);
});

it('should revert the node version to its original version when the upgrade checkbox is unchecked', async() => {
const setup = requiredSetup();

Expand Down
3 changes: 2 additions & 1 deletion pkg/eks/l10n/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ eks:
label: Tags
version:
label: Kubernetes Version
upgradeWarning: (minor versio n >1 not allowed by EKS)
upgradeWarning: (minor version >1 not allowed by EKS)
upgradeDisallowed: "EKS only allows upgrades of one minor version: you must upgrade all node group versions before upgrading the cluster version."
vpcSubnet:
label: VPC and Subnet
1 change: 1 addition & 0 deletions pkg/eks/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface EKSNodeGroup {
version?: string
__nameUnique?: boolean
_isNew?: boolean,
_isUpgrading: boolean
}

export interface EKSConfig {
Expand Down

0 comments on commit ba26cf5

Please sign in to comment.