KubeVirt extension PoC #888

vojtechszocs · 2018-11-30T19:33:51Z

This is a work in progress. Changes will be made based on received feedback.

Summary

This PR introduces KubeVirt extension of Console UI.

To avoid complexity and time spent on designing a formal extension mechanism, the current form of integration is through minimum necessary changes to application code. This PR therefore contains two types of code:

KubeVirt code, isolated to public/extend/kubevirt directory
- example: public/extend/kubevirt/components/vm-list.jsx
modification of application code, importing and using KubeVirt code
- example: public/components/resource-pages.ts

Code in public/extend/kubevirt directory contains program elements necessary for the integration, like the list and details page for the VirtualMachine resource. KubeVirt specific UI elements, like the "Create VM" wizard, are maintained in a separate repository and consumed via npm package:

GitHub repo: https://github.com/kubevirt/web-ui-components
Travis CI: https://travis-ci.org/kubevirt/web-ui-components
Coveralls: https://coveralls.io/github/kubevirt/web-ui-components
Cosmos playground: https://kubevirt.io/web-ui-components (build info)

React components in kubevirt-web-ui-components package generally follow Console UI dependency requirements, including PatternFly 3 and the corresponding PatternFly-React implementation. Files contained in this package include Babel-processed JavaScript and Sass partials.

Scope

VM list page

VM details page

KubeVirt version on all-projects overview page

VM list and details on project overview page

VM actions

What's missing from kubevirt-web-ui fork

Below features have been skipped on purpose in order to reduce initial PR complexity:

VM list page:
- missing create VM through wizard workflow
VM details page:
- missing detail tabs: Consoles, Network Interfaces, Disks
VM actions:
- missing "Restart Virtual Machine"

Environment setup

Here are the steps I've used to prepare a local development environment with KubeVirt enabled:

download the oc binary distributed via openshift-origin-client-tools archive for OKD v3.10.0 and put it on your PATH
- if you plan to run UI integration tests, create a kubectl symlink to oc
make sure your cluster is up, note the cluster up configuration directory:
```
ocBaseDir="/path/to/openshift.local.clusterup"
```

put your cluster down and update node/node-config.yaml:

sed -i "/kind/ a\kubeletArguments:\n  feature-gates:\n  - DevicePlugins=true" $ocBaseDir/node/node-config.yaml

put your cluster up and login as admin user:
```
oc login -u system:admin
```

prepare your cluster for KubeVirt installation:

KUBELET_ROOTFS=$(docker inspect $(docker ps | grep kubelet | cut -d" " -f1) | jq -r ".[0].GraphDriver.Data.MergedDir" -)
sudo mkdir -p /var/lib/kubelet/device-plugins $KUBELET_ROOTFS/var/lib/kubelet/device-plugins
sudo mount -o bind $KUBELET_ROOTFS/var/lib/kubelet/device-plugins /var/lib/kubelet/device-plugins

oc adm policy add-cluster-role-to-user cluster-admin developer
oc label node localhost kubevirt.io/schedulable=true
oc adm policy add-scc-to-user privileged system:serviceaccount:kube-system:kubevirt-privileged
oc adm policy add-scc-to-user privileged system:serviceaccount:kube-system:kubevirt-controller
oc adm policy add-scc-to-user privileged system:serviceaccount:kube-system:kubevirt-infra
oc adm policy add-scc-to-user privileged system:serviceaccount:kube-system:kubevirt-apiserver

cat <<EOF > dv-featuregate.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: kubevirt-config
  namespace: kube-system
  labels:
    kubevirt.io: ""
data:
  feature-gates: "DataVolumes"
EOF
oc create -f dv-featuregate.yaml
oc adm policy add-scc-to-user privileged -z cdi-sa

install KubeVirt v0.9.1 and CDI v1.2.0

oc create -f https://github.com/kubevirt/containerized-data-importer/releases/download/v1.2.0/cdi-controller.yaml
oc create -f https://github.com/kubevirt/kubevirt/releases/download/v0.9.1/kubevirt.yaml

add sample VM resource

oc apply -f https://raw.githubusercontent.com/kubevirt/demo/master/manifests/vm.yaml

Related resources

KubeVirt OpenAPI reference

Action items necessary for PR completion

improve KubeVirt code to use TypeScript
ensure that initial application chunk (main-chunk) is not impacted by KubeVirt code
analyze KubeVirt related chunks (virtual-machine-chunk) and try to reduce their footprint

coreos-ui · 2018-11-30T19:33:53Z

Can one of the admins verify this patch?

coreos-ui · 2018-11-30T19:33:54Z

Can one of the admins verify this patch?

vojtechszocs · 2018-11-30T20:02:14Z

cc @jelkosz @mareklibra @rawagner

@ohadlevy after talking with @spadgett, the current plan for KubeVirt integration with Console UI is through minimum necessary changes to application code. This approach is also used in kubevirt-web-ui fork, which should be discontinued in the long term.

The goal of this PR is to have some initial KubeVirt support in Console UI, which leaves a more formal extension mechanism up to follow-up discussion, taking both Console and KubeVirt goals into consideration.

spadgett · 2018-11-30T19:39:13Z

frontend/package.json

@@ -85,6 +86,7 @@
    "react-tagsinput": "3.19.x",
    "react-transition-group": "2.3.x",
    "react-virtualized": "9.x",
+    "reactabular-dnd": "8.x",


We should track down why this is needed. I don't think anything you've added should need this dependency.

Agreed, declaring an explicit dependency on reactabular-dnd shouldn't be necessary from target application's perspective. Adding only the kubevirt-web-ui-components one should be enough.

Please note that:

patternfly-react declares reactabular-table as a regular dependency (it's used to implement PF-React Table component)

kubevirt-web-ui-components uses reactabular-dnd which adds drag'n'drop support to Reactabular tables (to be used with PF-React Table component)

However, kubevirt-web-ui-components package currently doesn't declare reactabular-dnd as its dependency, which is a problem we're going to fix (along with some other dependency issues). See here for more details.

So with kubevirt-web-ui-components, we'll try not to introduce any additional peer dependencies (on top of existing ones that come from PF-React).

Currently, your dependency tree includes reactabular-table. After adding kubevirt-web-ui-components, it will also include reactabular-dnd. With webpack, code for reactabular-dnd should be (lazy) loaded only as part of KubeVirt specific chunk (assuming the main application doesn't use reactabular-dnd).

PRs for patternfly-react and kubevirt-web-ui-components projects posted, these should clean up dependencies and reduce warnings when calling yarn install.

@spadgett In order to eliminate unmet peer dependency warnings on lodash (coming mostly from patternfly-react's transitive deps), I'm planning to add "lodash": "npm:lodash-es" custom alias.

spadgett · 2018-11-30T19:40:25Z

frontend/public/components/overview/index.tsx

+      const buildConfigs = this.getBuildConfigsForResource(vm);
+      const services = this.getServicesForResource(vm);
+      const routes = this.getRoutesForServices(services);
+      const pods = this.getPodsForResource(vm);


Will VMs have these resources?

I am not too sure. For now, I simply wanted to comply with the OverviewItem contract.

@jelkosz @mareklibra can you please share your thoughts here?

Also note that I've changed

@@ -1008,7 +1040,7 @@ export type OverviewItem = { obj: K8sResourceKind; pods?: K8sResourceKind[]; previous?: PodControllerOverviewItem; - readiness: OverviewItemReadiness; + readiness?: OverviewItemReadiness; routes: K8sResourceKind[]; services: K8sResourceKind[]; };

in order to avoid rendering "X of Y pods" label on the right side of object list row.

In general, there is an initiative to make the VMs to be completely native. It means they should play well with all the features of kubernetes/openshift such as deployments, replica sets etc. We are not there yet but we are getting there.

For this particular objects:
buildconfig - there has been no effort yet to make this work nor to test it. So I would go with "it does not work" for this case
services: yes, plays well with VMs
routes: yes, plays well with VMs
pods: if the VM is running, there is a pod associated with it in which the qemu process is actually living. So yes.

buildconfig - there has been no effort yet to make this work nor to test it. So I would go with "it does not work" for this case

@jelkosz I can set buildConfigs to an empty array (i.e. remove the call to getBuildConfigsForResource function).

Still, OverviewItem.buildConfigs is currently not optional, so we'll end up with "No Build Configs found for this resource." item in the overview details pane anyway:

@vojtechszocs how big change it would be to enrich the infra to support conditionally not to render the particular resources?

@jelkosz It shouldn't be a big change, I'll try to do that as part of this PR.

spadgett · 2018-11-30T19:40:59Z

frontend/integration-tests/tests/crud.scenario.ts


 const K8S_CREATION_TIMEOUT = 15000;

+const getKindForCRD = (model: K8sKind) => `${model.apiGroup}~${model.apiVersion}~${model.kind}`;


Use referenceForModel

Importing the referenceForModel function here:

import { referenceForModel } from '../../public/module/k8s';

and calling it causes the following error:

E/launcher - Error: /home/vszocs/go/src/github.com/openshift/console/frontend/node_modules/lodash-es/lodash.js:10 export { default as add } from './add.js'; ^^^^^^ SyntaxError: Unexpected token export at new Script (vm.js:79:7) at createScript (vm.js:251:10) at Object.runInThisContext (vm.js:303:10) at Module._compile (internal/modules/cjs/loader.js:656:28) at Module._extensions..js (internal/modules/cjs/loader.js:699:10) at Object.require.extensions.(anonymous function) [as .js] (/home/vszocs/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:392:14) at Module.load (internal/modules/cjs/loader.js:598:32) at tryModuleLoad (internal/modules/cjs/loader.js:537:12) at Function.Module._load (internal/modules/cjs/loader.js:529:3) at Module.require (internal/modules/cjs/loader.js:636:17)

In public/module/k8s/k8s we do import * as _ from 'lodash-es' and in lodash-es/lodash.js there's ES6 style export:

export { default as add } from './add.js'

which causes this error. This is the reason why didn't use the referenceForModel function.

I can look into this problem and try to fix it.

Hmm, with Jest you're white-listing node_modules/lodash-es/* for module transformation (transformIgnorePatterns). Anything else in node_modules is expected to be already pre-compiled and is therefore excluded from transformation by ts-jest.

I guess we'll need to apply a similar approach when running Protractor. This issue is basically Protractor unable to parse node_modules/lodash-es/lodash.js which is referenced by application code (public/module/k8s/k8s.ts) that we're trying to reuse.

OK, I see now that integration tests actually use lodash instead of lodash-es 😃

I've tried to use -I 'node_modules/(?!lodash-es/.*)' (custom ignore pattern) argument for ts-node within the test-suite script, but ran into some weird issues.

So for integration tests specifically, we'll need to map lodash-es to lodash in terms of TypeScript module resolution.

I've solved the issue of aliasing lodash-es to lodash in context of integration tests.

Doing that, I've run into another issue - access to window global which is undefined when being evaluated in context of integration tests (Jasmine) as part of the import process, for example:

// public/module/k8s/k8s.ts export const k8sBasePath = `${(window as any).SERVER_FLAGS.basePath}api/kubernetes`;

I've solved that issue as well, mocking window.SERVER_FLAGS object as necessary.

It's now possible to import any code from public directory within integration test code, so I'll go ahead and use the referenceForModel function as suggested by @spadgett.

spadgett · 2018-11-30T19:43:23Z

frontend/public/extend/kubevirt/components/modals/index.js

@@ -0,0 +1,2 @@
+export const startStopVmModal = (props) => import('./start-stop-vm-modal' /* webpackChunkName: "start-stop-vm-modal" */)


Keeps acronyms capitalized

Suggested change

export const startStopVmModal = (props) => import('./start-stop-vm-modal' /* webpackChunkName: "start-stop-vm-modal" */)

export const startStopVMModal = (props) => import('./start-stop-vm-modal' /* webpackChunkName: "start-stop-vm-modal" */)

Agreed, will do.

spadgett · 2018-11-30T19:43:44Z

frontend/public/extend/kubevirt/components/modals/start-stop-vm-modal.jsx

+import { createModalLauncher, ModalTitle, ModalBody, ModalSubmitFooter } from '../../../../components/factory/modal';
+import { k8sPatch } from '../../../../module/k8s';
+
+class StartStopVmModal extends PromiseComponent {


Suggested change

class StartStopVmModal extends PromiseComponent {

class StartStopVMModal extends PromiseComponent {

Agreed, will do.

spadgett · 2018-11-30T21:05:46Z