Skip to content

Conversation

@abhinandan13jan
Copy link
Contributor

@abhinandan13jan abhinandan13jan commented Jan 14, 2020

This PR addresses https://issues.redhat.com/browse/ODC-2698

  • Obtain capability of an operator backed object from CSV annotation and add it as a key capabilityLevel to an item of kind installedOperator
  • Add capabilty filters to getFilter function
  • Adjust filterGroup, filterValueMap to accommodate capability filters and reflect as desired in Dev-Catalog.
  • Set default filter set to true for Auto Pilot, Deep Insights and Full Lifecycle
  • Added and adjusted tests in catalog.spec.tsx
  • Convert catalog-item.jsx to tsx
  • Add typings

Screencast from 01-16-2020 04_48_30 PM

TO-DO: Add typings for catalog-items.tsx

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. component/core Related to console core functionality component/olm Related to OLM labels Jan 14, 2020
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why this has been changed? Also, I don't see this prop being used anywhere in tile-view-page

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved this out....

@andrewballantyne
Copy link
Contributor

/kind feature

@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 15, 2020
@abhinandan13jan abhinandan13jan changed the title [WIP]feat(dev-catalog): Add Capability level filters feat(dev-catalog): Add Capability level filters Jan 16, 2020
@openshift-ci-robot 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 Jan 16, 2020
@debsmita1
Copy link
Contributor

@abhinandan13jan You would need to remove the log statements from your PR.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 16, 2020
@abhinandan13jan abhinandan13jan changed the title feat(dev-catalog): Add Capability level filters [WIP]feat(dev-catalog): Add Capability level filters Jan 16, 2020
@openshift-ci-robot 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 Jan 16, 2020
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no type ? in the jsx file it was declared as an array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had left a TODO: remark that I am left to update the typings. I've updated the typings in all places possible. Please re-review :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't notice your TODO remark below the GIF yesterday :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a type here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup. Added

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the log statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2020
@abhinandan13jan abhinandan13jan changed the title [WIP]feat(dev-catalog): Add Capability level filters feat(dev-catalog): Add Capability level filters Jan 17, 2020
@openshift-ci-robot 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 Jan 17, 2020
@debsmita1
Copy link
Contributor

please fix the lint error

@abhinandan13jan
Copy link
Contributor Author

/test backend
/test e2e-gcp-console

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix the comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please write it like :

Suggested change
openOverlay(detailsItem: Item): void {
openOverlay = (detailsItem: Item): void => {

and get rid of this.openOverlay = this.openOverlay.bind(this);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI you don't need a type for assignments. Only when the right side is ambiguous like {...}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unnecessary as you do not need a return statement for a single line function.

Copy link
Contributor Author

@abhinandan13jan abhinandan13jan Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.... I console logged the csv and forgot to remove the {}.. will remove this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about assignment types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these values end up in the url? They should probably be lower case be no spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, they do... they cannot be case changes or altered directly as they need to be an exact string match with the annotation.

  value: 'BasicInstall' || 'basicinstall' will not match with metadata.annotations.cabalities: Basic Install

Following the above recommendation, I am changing the vales to lowercase and also transforming the the capability data obtained from the CSV while adding it to the mock object
in dev-catalog.ts, line 59

@abhinandan13jan abhinandan13jan changed the title feat(dev-catalog): Add Capability level filters [WIP]feat(dev-catalog): Add Capability level filters Jan 24, 2020
@openshift-ci-robot 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 Jan 24, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020
@abhinandan13jan abhinandan13jan changed the title [WIP]feat(dev-catalog): Add Capability level filters feat(dev-catalog): Add Capability level filters Jan 24, 2020
@openshift-ci-robot 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 Jan 24, 2020
@debsmita1
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020
Comment on lines 29 to 28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this needs a revert @abhinandan13jan

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, reverted

Use dependecies from vendor dir

Add goflag in build-backend

backend: disable HTTP/2

Our proxy can't handle WebSockets when using HTTP/2.

Increased height of activity card in overview dashboard

Signed-off-by: Afreen Rahman <afrahman@redhat.com>

add unit tests for ApplicationGroupResource

refactor code

refactor TopologyApplicationResource

use isEmptyRender instead of exists

simplify component

Added helm chart repo proxy to the console backend

All requests to `/api/helm/charts/*` are forwarded to the declared Helm chart repository.
By default this is `https://redhat-developer.github.com/redhat-helm-charts`, but configurable via

- `helm-chart-repo-url` CLI argument
- BRIDGE_HELM_CHART_REPO_URL env variable
- `config.yaml`:

```
.
.
.
helm:
  chartRepository:
	url: https://foo.bar
	caFile: /path/to/cert.file

```

If needed, certificates for accessing the repo can be specified via `helm-chart-repo-ca-file` CLI arg, caFile config param, or using
`BRIDGE_HELM_CHART_REPO_CA_FILE` env variable. If not specified, the system root CA will be used.

Make extra step in VM Wizard to procceed trough Virtual Hardware tab

Fix modal close on confirmation of a node regroup

Add 4.4 channels

bump formik

- fixes enableReinitialize

shared: make alert props optional in FormFooter

shared: add basic methods and a method for adding multiple object keys to PatchBuilder

Monitoring Dashboards: Change "Refresh Interval" options

Change some of the "Refresh Interval" dropdown options and also add an
option for switching auto-refresh off.

adds monitoring metrics component and legend

Custom pages added for noobaa

chore: bump builder image dependencies

* go 1.13
* node 13.6.0
* yarn 1.21.1
* kubectl 1.17.1

update table filter

Add shadow on hover for topology decorators

Initial boot order tests

feat(topology-resourcemenu): add resource menu actions

Allow children components on DetailsPage header

Remove cluster operator upgrade activity

BaremetalHost: Use new errorType field in Host CR

When the operational status is in error, the BaremetalHost operator now
reports the type of error from a fixed list. This is more reliable than
determining the type of error from the provisioning status (e.g. a
registration error may occur while the host is provisioning).

The possible error types are registration, inspection, provisioning, and
power management errors. There is no longer a validation error, and has
not been for some time.

Currently, there are actually separate provisioning states for each
error type, so the Host would never have been in a Registering or
Provisioning state after an error anyway, and this code would always
have reported the host status as just 'error'. These separate states are
being removed in favour of the separate errorType field. Since the
existing code was not reachable, there is no need to keep it for
backward compatibility with older versions of the operator.

Signed-off-by: Zane Bitter <zbitter@redhat.com>

make k8sPatch resilient to arrays with null values

shared: move compareOwnerReferences from kubevirt

enhance FirehoseResult type

Add List component for detail properties

This adds common component for lists in detail page properties.
- Common styling
- Semantic use of lists instead of spans and brs

Add monitoring dashboard graph

Refactoring: useReceiverFormStore and move single used functions out of utils

add cpu pinning modal and functionality

dashboards: initial support for table cards

[metal3] Use HrefNavItem for BMH nav link

This ensures that namespace is switched to 'openshift-machine-api'
which is expected by BMO. This behaviour aligns with Machines
namespace handling.

Related Kubernetes issue: kubernetes/kubernetes#65551

kubevirt: enhance memory validation and template selection

- unify and enhance template selections for validation and create requests
- use GiB instead of GB for memory to allign with validations
- show validation intervals

Add the component and utility for helm release group

initial implementation

initial implementation

remove styles for selected and fine tune

initial implementation

align the item to UX designs

add tests for utility and update test

use fill attribute istead of styles

update snapshot

add corner radius to helm icon

refactor

disable dnd for helm release node

decrease opacity on filtered items

fix llint error

lint and destructuring

Add edit application from deploy image

Add unit tests for edit-application-utils

Set image-stream values on populating the form with app data

Fix bug wherein for image from internal registry new imagestream was created

Change page title on Admin > Cluster Settings > Alertmanager

Bug https://bugzilla.redhat.com/show_bug.cgi?id=1789946

Minor Enhancements for Stoarge Dashboards

As per UXD suggestions, made these minor enhancements:

* Fixed width of bar in breakdown card
* Chnaging tab names
* Spanning of lines to full width in utilization card
* Linking of Nodes

Changed ceph plugin gating from cephcluster model to storagecluster model

Signed-off-by: Afreen Rahman <afrahman@redhat.com>

---------- Merged Pull request afreen23#1 -------------
Adding filter for nodes in ocs inventory

Signed-off-by: Ankush Behl <cloudbehl@gmail.com>
------------------

Co-authored-by: Ankush Behl <cloudbehl@gmail.com>

Reconcile feature flag extensions

Clean up baremetal host states

- Define bare metal host status and errors descriptions in constants
- Pass status description as part of getBareMetalHostStatus helper
- Simplify BareMetalHostStatus logic
- Render status descriptions in status popovers
- Render error status descriptions in status popover together with
  detailed error message
- Remove unused bare metal host states

Convert inline button to us isInline and proper spacing

chore: bump node-sass to 4.13.1

Required for Node.js 13.

add Deprovision action to Bare Metal Hosts

Add clickable action to monitoring dashboard graph

adds promQL editor and toggle option for it

dashboard status: differentiate between unknown and not available states

Add useExtensions hook & withExtensions HOC, use it for NavItem ext.

Unify runtime handling of gated and always-on extensions

Update login tests to work with either PF3 or PF4 login pages

Added owners file to ceph and noobaa int

Add support for Helm Install with release form and backend API

Rename Overview pages to Details pages
Rename Dashboards page to Overview
Rename Overview Dashboard page to Cluster

Add pause/unpause VM feature

Monitoring Dashboards: Add currently displayed dashboard to URL

Also changes the layout of the dropdown options to bring it closer to
the mocks.

Now clears any dashboard data as soon as the current dashboard is
changed, so that all cards from the previous dashboard are immediately
cleared.

Added expand and upgrade activities in persistent storage dashboard

Signed-off-by: Afreen Rahman <afrahman@redhat.com>

All long string container names to wrap and add spacing to checkbox rows

Monitoring Dashboards: Add graph legends

Also includes some graph card style fixes to remove unnecessary margins
and padding.

kubevirt: fix modal footer padding

kubevirt: Add RDP L2 integration test

Configuration of nodes for the L2 network is out of scope (means bridge and DHCP server).

A fake Windows VM (Fedora inside) is created, qemu-guest-agent installed, static IP address set
to be later checked to verify the expected flow from the guest agent reporting to RDP console UI.

Revert "chore: bump builder image dependencies"

This reverts commit ef62018.

chore: update to go 1.13

add ImageManifestVuln list/detail views

Added installation flow for external cluster

tracker

track1

tracker1

tracker2

track2

tracker2

track

trc

trc
@christianvogt
Copy link
Contributor

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020
Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, andrewballantyne, christianvogt, debsmita1, jeff-phillips-18

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2020
@andrewballantyne
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member

/hold
for merge queue fix #4065

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 24, 2020
@spadgett
Copy link
Member

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2020
@openshift-merge-robot openshift-merge-robot merged commit dd15f71 into openshift:master Jan 25, 2020
@spadgett spadgett added this to the v4.4 milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/backend Related to backend component/ceph Related to ceph-storage-plugin component/core Related to console core functionality component/dashboard Related to dashboard component/dev-console Related to dev-console component/knative Related to knative-plugin component/kubevirt Related to kubevirt-plugin component/metal3 Related to metal3-plugin component/monitoring Related to monitoring component/network-attachment-definition Related to network-attachment-definition component/noobaa Related to noobaa-storage-plugin component/olm Related to OLM component/sdk Related to console-plugin-sdk component/shared Related to console-shared kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants