filter incompatible helm charts from dev catalog #5767

debsmita1 · 2020-06-17T17:07:03Z

JIRA Story:
https://issues.redhat.com/browse/ODC-4062

Solution Description:

filters the helm charts in the dev-catalog that are incompatible with the Kubernetes Version
filters helm chart versions in the Chart Version dropdown that are incompatible with the Kubernetes Version
Fixed the type of HelmChart (Referred the Helm docs: https://helm.sh/docs/topics/charts/ , section : The Chart.yaml File )
Added tests

debsmita1 · 2020-06-17T17:08:22Z

/assign @rohitkrai03

abhi-kn · 2020-06-18T09:08:27Z

/retest

abhinandan13jan · 2020-06-18T11:11:04Z

frontend/public/components/catalog/catalog-page.tsx

+      _.forEach(helmCharts, (obj: HelmChart) => {
+        if (obj?.kubeVersion && semver.valid(kubernetesVersion)) {
+          if (semver.satisfies(kubernetesVersion, obj.kubeVersion)) {
+            arr = [...arr, obj];


why not push? Does it fail a shallow compare somewhere?

+1 for add.push here.
And just one addition when you accept this change: Give things more meaningful names, like const filteredCharts instead of arr and maybe helmChart instead of just obj? :)

abhinandan13jan · 2020-06-18T11:17:07Z

frontend/public/components/catalog/catalog-page.tsx

+            arr = [...arr, obj];
+          }
+        } else {
+          arr = [...arr, obj];


can it be clubbed with above arr = [ ...arr, obj ] statement using || ?

Use add.push(obj) here as well. I personally like this else case :)

@abhinandan13jan the purpose of adding this in else is if a helm chart doesn't have kubeVersion then consider it as compatible

jerolimov · 2020-06-18T12:11:04Z

frontend/public/components/catalog/catalog-page.tsx

-      setHelmCharts(json.entries);
+      let kubernetesVersion: string = '';
+      await k8sVersion()
+        .then((response) => {


I personally would prefer not to mix async/await with .then/.catch. Could you use a try catch block here instead? What do you think? Because this is personal preference I would also accept this as it is.

added try-catch block

jerolimov · 2020-06-18T12:12:02Z

frontend/public/components/catalog/catalog-page.tsx

+            arr = [...arr, obj];
+          }
+        } else {
+          arr = [...arr, obj];


Use add.push(obj) here as well. I personally like this else case :)

jerolimov · 2020-06-18T12:18:43Z

frontend/public/components/catalog/catalog-page.tsx

+      _.forEach(helmCharts, (obj: HelmChart) => {
+        if (obj?.kubeVersion && semver.valid(kubernetesVersion)) {
+          if (semver.satisfies(kubernetesVersion, obj.kubeVersion)) {
+            arr = [...arr, obj];


+1 for add.push here.
And just one addition when you accept this change: Give things more meaningful names, like const filteredCharts instead of arr and maybe helmChart instead of just obj? :)

jerolimov

/lgtm

debsmita1 · 2020-06-18T17:55:20Z

/test images

rohitkrai03 · 2020-06-18T19:36:56Z

frontend/public/components/catalog/catalog-page.tsx

+      try {
+        const response = await k8sVersion();
+        kubernetesVersion = getK8sGitVersion(response) || '-';
+      } catch {
+        kubernetesVersion = 'unknown';
+      }


Move this logic outside into a separate effect.

rohitkrai03 · 2020-06-18T19:39:16Z

frontend/public/components/catalog/catalog-page.tsx

+      const filteredHelmChartEntries = filterIncompatibleHelmCharts(
+        json.entries,
+        kubernetesVersion,
+      );


Why do you have the filtering mechanism separately? While normalizing the helm charts we already go through all the helm charts and reduce it into items for catalog. You can re-use that same reduce to do the filtering easily. We also do filtering of chart versions there. Later on we would have so many helm charts that this would come out to be really expensive.

I would like to have the filtering based on kubeVersion to be along with this logic - #5699

rohitkrai03

/lgtm

debsmita1 · 2020-06-22T08:01:30Z

/assign @christianvogt

sahil143 · 2020-06-22T17:14:57Z

frontend/packages/dev-console/src/components/helm/form/HelmChartVersionDropdown.tsx

      setHelmChartEntries(_.get(json, ['entries', chartName]));
-      setHelmChartVersions(getChartVersions(_.get(json, ['entries', chartName])));
+      setHelmChartVersions(
+        getChartVersions(_.get(json, ['entries', chartName]), kubernetesVersion),
+      );


lets change this to use optional chaining

christianvogt · 2020-06-22T18:46:23Z

/lgtm cancel

christianvogt · 2020-06-22T19:05:43Z

/lgtm
/approve

openshift-ci-robot · 2020-06-22T19:06:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, debsmita1, jerolimov, rohitkrai03

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [christianvogt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

debsmita1 · 2020-06-30T15:16:19Z

/kind feature

openshift-ci-robot added the component/core Related to console core functionality label Jun 17, 2020

openshift-ci-robot requested review from spadgett and zherman0 June 17, 2020 17:07

openshift-ci-robot assigned rohitkrai03 Jun 17, 2020

debsmita1 changed the title ~~filter incompatible helm charts from dev catalog~~ [WIP] filter incompatible helm charts from dev catalog Jun 17, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2020

debsmita1 force-pushed the helm-kube-version branch from 5fb8264 to 18b069a Compare June 18, 2020 08:20

debsmita1 changed the title ~~[WIP] filter incompatible helm charts from dev catalog~~ filter incompatible helm charts from dev catalog Jun 18, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2020

abhinandan13jan reviewed Jun 18, 2020

View reviewed changes

jerolimov reviewed Jun 18, 2020

View reviewed changes

debsmita1 force-pushed the helm-kube-version branch from 18b069a to 7e7af31 Compare June 18, 2020 14:38

jerolimov approved these changes Jun 18, 2020

View reviewed changes

openshift-ci-robot assigned jerolimov Jun 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2020

rohitkrai03 suggested changes Jun 18, 2020

View reviewed changes

debsmita1 force-pushed the helm-kube-version branch from 7e7af31 to 2db07b9 Compare June 19, 2020 10:15

openshift-ci-robot added component/dev-console Related to dev-console and removed lgtm Indicates that a PR is ready to be merged. labels Jun 19, 2020

debsmita1 force-pushed the helm-kube-version branch from 2db07b9 to c153f83 Compare June 19, 2020 11:32

rohitkrai03 approved these changes Jun 19, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 19, 2020

openshift-ci-robot assigned christianvogt Jun 22, 2020

spadgett removed their request for review June 22, 2020 13:58

sahil143 reviewed Jun 22, 2020

View reviewed changes

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2020

filter incompatible helm charts from dev catalog

e532c33

debsmita1 force-pushed the helm-kube-version branch from c153f83 to e532c33 Compare June 22, 2020 19:03

debsmita1 requested a review from sahil143 June 22, 2020 19:04

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2020

openshift-merge-robot merged commit 55c7ea0 into openshift:master Jun 22, 2020

spadgett added this to the v4.6 milestone Jun 24, 2020

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 30, 2020

debsmita1 mentioned this pull request Sep 8, 2020

Bug 1876741: undo filtering of incompatible helm charts #6550

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

filter incompatible helm charts from dev catalog #5767

filter incompatible helm charts from dev catalog #5767

debsmita1 commented Jun 17, 2020 •

edited

debsmita1 commented Jun 17, 2020

abhi-kn commented Jun 18, 2020

abhinandan13jan Jun 18, 2020 •

edited

jerolimov Jun 18, 2020

abhinandan13jan Jun 18, 2020 •

edited

jerolimov Jun 18, 2020

debsmita1 Jun 18, 2020

jerolimov Jun 18, 2020 •

edited

debsmita1 Jun 18, 2020

jerolimov Jun 18, 2020

jerolimov Jun 18, 2020

jerolimov left a comment

debsmita1 commented Jun 18, 2020

rohitkrai03 Jun 18, 2020

rohitkrai03 Jun 18, 2020

rohitkrai03 Jun 18, 2020

rohitkrai03 left a comment

debsmita1 commented Jun 22, 2020

sahil143 Jun 22, 2020

christianvogt commented Jun 22, 2020

christianvogt commented Jun 22, 2020

openshift-ci-robot commented Jun 22, 2020

debsmita1 commented Jun 30, 2020

filter incompatible helm charts from dev catalog #5767

filter incompatible helm charts from dev catalog #5767

Conversation

debsmita1 commented Jun 17, 2020 • edited

debsmita1 commented Jun 17, 2020

abhi-kn commented Jun 18, 2020

abhinandan13jan Jun 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinandan13jan Jun 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov Jun 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerolimov left a comment

Choose a reason for hiding this comment

debsmita1 commented Jun 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 left a comment

Choose a reason for hiding this comment

debsmita1 commented Jun 22, 2020

Choose a reason for hiding this comment

christianvogt commented Jun 22, 2020

christianvogt commented Jun 22, 2020

openshift-ci-robot commented Jun 22, 2020

debsmita1 commented Jun 30, 2020

debsmita1 commented Jun 17, 2020 •

edited

abhinandan13jan Jun 18, 2020 •

edited

abhinandan13jan Jun 18, 2020 •

edited

jerolimov Jun 18, 2020 •

edited