Skip to content

Commit

Permalink
Fix for Monitoring Helm chart sometimes adds empty selector (rancher#…
Browse files Browse the repository at this point in the history
…10085)

* Fix for Monitoring Helm chart sometimes adds empty selector

* Correct close icon styling

* Tweak close icon width

* Fix lint issues
  • Loading branch information
nwmac authored Dec 14, 2023
1 parent 83550bd commit 29d8f34
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 2 deletions.
4 changes: 4 additions & 0 deletions cypress/e2e/po/components/kubectl.po.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export default class Kubectl extends ComponentPo {
return this;
}

closeTerminal() {
this.self().get('[data-testid="wm-tab-close-button"]').click();
}

/**
*
* @param command Kube command without the 'kubectl'
Expand Down
44 changes: 44 additions & 0 deletions cypress/e2e/tests/pages/charts/prometheus.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('Charts', { tags: ['@charts', '@adminUser'] }, () => {

before(() => {
cy.login();
cy.viewport(1280, 720);
chartsPage.goTo();
});

Expand All @@ -35,10 +36,14 @@ describe('Charts', { tags: ['@charts', '@adminUser'] }, () => {
terminal.executeCommand(`apply -f https://raw.githubusercontent.com/rancher/local-path-provisioner/${ provisionerVersion }/deploy/local-path-storage.yaml`)
.executeCommand('create -f https://raw.githubusercontent.com/rancher/local-path-provisioner/master/examples/pvc/pvc.yaml')
.executeCommand('create -f https://raw.githubusercontent.com/rancher/local-path-provisioner/master/examples/pod/pod.yaml');

terminal.closeTerminal();
});

// Don't actually install the chart, just navigate to the edit options page
beforeEach(() => {
cy.login();
chartsPage.goTo();
cy.intercept('POST', 'v1/catalog.cattle.io.clusterrepos/rancher-charts?action=install').as('prometheusChartCreation');
});

Expand All @@ -50,6 +55,9 @@ describe('Charts', { tags: ['@charts', '@adminUser'] }, () => {

const enableStorageCheckbox = new CheckboxPo('[data-testid="checkbox-chart-enable-persistent-storage"]');

// Scroll into view
enableStorageCheckbox.checkVisible();

enableStorageCheckbox.set();

const labeledSelectPo = new LabeledSelectPo('[data-testid="select-chart-prometheus-storage-class"]');
Expand All @@ -69,6 +77,42 @@ describe('Charts', { tags: ['@charts', '@adminUser'] }, () => {
expect(monitoringChart.values.prometheus).to.deep.equal(prometheusSpec.values.prometheus);
});
});

// Regression test for: https://github.com/rancher/dashboard/issues/10016
it('Should not include empty prometheus selector when installing (add/remove selector).', () => {
const tabbedOptions = new TabbedPo();

// Set prometheus storage class
chartsPage.goToInstall().nextPage().editOptions(tabbedOptions, '[data-testid="btn-prometheus"');

const enableStorageCheckbox = new CheckboxPo('[data-testid="checkbox-chart-enable-persistent-storage"]');

// Scroll into view
enableStorageCheckbox.checkVisible();

enableStorageCheckbox.set();

const labeledSelectPo = new LabeledSelectPo('[data-testid="select-chart-prometheus-storage-class"]');

labeledSelectPo.toggle();
labeledSelectPo.clickOptionWithLabel('local-path');

// Add a selector and then remove it - previously this would result in the empty selector being present
chartsPage.self().find(`[data-testid="input-match-expression-add-rule"]`).click();
chartsPage.self().find(`[data-testid="input-match-expression-remove-control-0"]`).click();

// Click on YAML. In YAML mode, the prometheus selector is present but empty
// It should not be sent to the API
chartsPage.editYaml();

chartsPage.installChart();

cy.wait('@prometheusChartCreation', { requestTimeout: 10000 }).then((req) => {
const monitoringChart = req.request?.body.charts.find((chart: any) => chart.chartName === 'rancher-monitoring');

expect(monitoringChart.values.prometheus).to.deep.equal(prometheusSpec.values.prometheus);
});
});
});
});
});
17 changes: 17 additions & 0 deletions shell/chart/monitoring/prometheus/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,23 @@ export default {
this.$set(storageSpec.selector, 'matchLabels', matchLabels);
this.$set(storageSpec.selector, 'matchExpressions', matchExpressions);
// Remove an empty selector object if present
// User can add a selector and then remove the selector - this will leave an empty structure as above
// We want to ensure we remove .selector.matchExpressions, .selector.matchLabels, and .selector if empty
// See: https://github.com/rancher/dashboard/issues/10016
if (storageSpec.selector.matchExpressions?.length === 0) {
delete storageSpec.selector.matchExpressions;
}
if (storageSpec.selector.matchLabels && Object.keys(storageSpec.selector.matchLabels).length === 0) {
delete storageSpec.selector.matchLabels;
}
if (Object.keys(storageSpec.selector).length === 0) {
delete storageSpec.selector;
}
},
},
};
Expand Down
13 changes: 11 additions & 2 deletions shell/components/nav/WindowManager/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ export default {
/>
<span class="tab-label"> {{ tab.label }}</span>
<i
class="closer icon icon-fw icon-x"
data-testid="wm-tab-close-button"
class="closer icon icon-fw icon-x wm-closer-button"
@click.stop="close(tab.id)"
/>
</div>
Expand Down Expand Up @@ -432,9 +433,16 @@ export default {
margin-left: 5px;
border: 1px solid var(--body-text);
border-radius: var(--border-radius);
line-height: 12px;
font-size: 10px;
width: 14px;
align-self: center;
display: flex;
justify-content: center;
&:hover {
background-color: var(--wm-closer-hover-bg);
border-color: var(--link-border);
color: var(--link-border);
}
}
}
Expand Down Expand Up @@ -494,4 +502,5 @@ export default {
border-right: var(--nav-border-size) solid var(--nav-border);
}
}
</style>

0 comments on commit 29d8f34

Please sign in to comment.