Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

v2v-conversion-pod: Use volumeDevices for Block volumeMode #557

Merged
merged 2 commits into from
Oct 16, 2019
Merged

v2v-conversion-pod: Use volumeDevices for Block volumeMode #557

merged 2 commits into from
Oct 16, 2019

Conversation

mareklibra
Copy link
Contributor

@mareklibra mareklibra force-pushed the bz1756328.conversionPodBlockMode branch from 36ba5ce to eb0e3ee Compare October 11, 2019 12:40
@mareklibra
Copy link
Contributor Author

The conversion pod created with volumeMode: Filesystem:

$ oc get pod kubevirt-v2v-conversion-wfsq4 -o yaml

apiVersion: v1
kind: Pod
metadata:
  annotations:
    openshift.io/scc: linux-bridge
  creationTimestamp: 2019-10-11T12:15:08Z
  generateName: kubevirt-v2v-conversion-
  name: kubevirt-v2v-conversion-wfsq4
  namespace: mlibra
  ownerReferences:
  - apiVersion: kubevirt.io/v1alpha3
    blockOwnerDeletion: true
    kind: VirtualMachine
    name: av2v-cnv-rhel7-mini
    uid: c57c4e93-ec20-11e9-8507-52540022ca9f
  resourceVersion: "3264051"
  selfLink: /api/v1/namespaces/mlibra/pods/kubevirt-v2v-conversion-wfsq4
  uid: c44eb3e2-ec20-11e9-8507-52540022ca9f
spec:
  containers:
  - image: quay.io/kubevirt/kubevirt-v2v-conversion:v2.0.0
    imagePullPolicy: IfNotPresent
    name: kubevirt-v2v-conversion
    resources: {}
    securityContext:
      privileged: true
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /data/input
      name: configuration
    - mountPath: /dev/kvm
      name: kvm
    - mountPath: /opt/vmware-vix-disklib-distrib
      name: volume-vddk
    - mountPath: /data/vm/disk1
      name: disk0
    - mountPath: /var/tmp
      name: v2v-conversion-temp
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kubevirt-v2v-conversion-7lpx5-token-mwq9q
      readOnly: true
  dnsPolicy: ClusterFirst
  enableServiceLinks: true
  imagePullSecrets:
  - name: kubevirt-v2v-conversion-7lpx5-dockercfg-dkmn5
  initContainers:
  - image: quay.io/mareklibra/mylib:v670
    imagePullPolicy: IfNotPresent
    name: vddk-init
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /opt/vmware-vix-disklib-distrib
      name: volume-vddk
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kubevirt-v2v-conversion-7lpx5-token-mwq9q
      readOnly: true
  priority: 0
  restartPolicy: Never
  schedulerName: default-scheduler
  securityContext: {}
  serviceAccount: kubevirt-v2v-conversion-7lpx5
  serviceAccountName: kubevirt-v2v-conversion-7lpx5
  terminationGracePeriodSeconds: 30
  tolerations:
  - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  volumes:
  - name: configuration
    secret:
      defaultMode: 420
      secretName: kubevirt-v2v-conversion-g7wpv
  - hostPath:
      path: /dev/kvm
      type: ""
    name: kvm
  - emptyDir: {}
    name: volume-vddk
  - name: disk0
    persistentVolumeClaim:
      claimName: disk0-2jkjs
  - name: v2v-conversion-temp
    persistentVolumeClaim:
      claimName: v2v-conversion-temp-fv8t8
  - name: kubevirt-v2v-conversion-7lpx5-token-mwq9q
    secret:
      defaultMode: 420
      secretName: kubevirt-v2v-conversion-7lpx5-token-mwq9q
status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: 2019-10-11T12:15:08Z
    message: pod has unbound immediate PersistentVolumeClaims (repeated 2 times)
    reason: Unschedulable
    status: "False"
    type: PodScheduled
  phase: Pending
  qosClass: BestEffort

@mareklibra
Copy link
Contributor Author

The conversion pod created with volumeMode: Block, but v2v-conversion-temp still uses default storage class with Filesystem volumeMode:

$ oc get pod kubevirt-v2v-conversion-kq4tw -o yaml

apiVersion: v1
kind: Pod
metadata:
  annotations:
    openshift.io/scc: linux-bridge
  creationTimestamp: 2019-10-11T12:36:10Z
  generateName: kubevirt-v2v-conversion-
  name: kubevirt-v2v-conversion-kq4tw
  namespace: mlibra
  ownerReferences:
  - apiVersion: kubevirt.io/v1alpha3
    blockOwnerDeletion: true
    kind: VirtualMachine
    name: av2v-cnv-rhel7-mini-block2
    uid: b6373316-ec23-11e9-b9de-5254003e64e0
  resourceVersion: "3274094"
  selfLink: /api/v1/namespaces/mlibra/pods/kubevirt-v2v-conversion-kq4tw
  uid: b47e2afc-ec23-11e9-8507-52540022ca9f
spec:
  containers:
  - image: quay.io/kubevirt/kubevirt-v2v-conversion:v2.0.0
    imagePullPolicy: IfNotPresent
    name: kubevirt-v2v-conversion
    resources: {}
    securityContext:
      privileged: true
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeDevices:
    - devicePath: /data/vm/disk1
      name: disk0
    volumeMounts:
    - mountPath: /data/input
      name: configuration
    - mountPath: /dev/kvm
      name: kvm
    - mountPath: /opt/vmware-vix-disklib-distrib
      name: volume-vddk
    - mountPath: /var/tmp
      name: v2v-conversion-temp
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kubevirt-v2v-conversion-gkltv-token-hrqkw
      readOnly: true
  dnsPolicy: ClusterFirst
  enableServiceLinks: true
  imagePullSecrets:
  - name: kubevirt-v2v-conversion-gkltv-dockercfg-tdqm6
  initContainers:
  - image: quay.io/mareklibra/mylib:v670
    imagePullPolicy: IfNotPresent
    name: vddk-init
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /opt/vmware-vix-disklib-distrib
      name: volume-vddk
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kubevirt-v2v-conversion-gkltv-token-hrqkw
      readOnly: true
  priority: 0
  restartPolicy: Never
  schedulerName: default-scheduler
  securityContext: {}
  serviceAccount: kubevirt-v2v-conversion-gkltv
  serviceAccountName: kubevirt-v2v-conversion-gkltv
  terminationGracePeriodSeconds: 30
  tolerations:
  - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  volumes:
  - name: configuration
    secret:
      defaultMode: 420
      secretName: kubevirt-v2v-conversion-22t96
  - hostPath:
      path: /dev/kvm
      type: ""
    name: kvm
  - emptyDir: {}
    name: volume-vddk
  - name: disk0
    persistentVolumeClaim:
      claimName: disk0-pq7rk
  - name: v2v-conversion-temp
    persistentVolumeClaim:
      claimName: v2v-conversion-temp-lp4df
  - name: kubevirt-v2v-conversion-gkltv-token-hrqkw
    secret:
      defaultMode: 420
      secretName: kubevirt-v2v-conversion-gkltv-token-hrqkw
status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: 2019-10-11T12:36:10Z
    message: pod has unbound immediate PersistentVolumeClaims (repeated 2 times)
    reason: Unschedulable
    status: "False"
    type: PodScheduled
  phase: Pending
  qosClass: BestEffort

@mareklibra
Copy link
Contributor Author

Cc @nyoxi

Can you please have a look at the results for Block/Filesystem volumeModes above? They look good to me.

@mareklibra
Copy link
Contributor Author

The v2v-conversion-temp is recently expected to be mounted to /var/tmp.
It is being created on default storage class.

If it's storage class defaults to volumeMode: Block (instead of so far tested Filesystem), can the conversion pod deal with it? The block device is still being added to the /var/tmp. Shouldn't we use /dev/something instead and handle that case in the conversion pod?

I do not see this happening in oVirt/v2v-conversion-host@cfe8e00 .

@nyoxi,, wdyt?

@mareklibra mareklibra force-pushed the bz1756328.conversionPodBlockMode branch from eb0e3ee to 66831d9 Compare October 11, 2019 12:56
@coveralls
Copy link

coveralls commented Oct 11, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 2379

  • 3 of 11 (27.27%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 82.431%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/k8s/requests/v2v/importVmware.js 2 10 20.0%
Files with Coverage Reduction New Missed Lines %
src/components/Wizard/CreateVmWizard/stateUpdate/providers/prefillVmStateUpdate.js 1 8.57%
src/k8s/requests/v2v/importVmware.js 2 10.34%
Totals Coverage Status
Change from base Build 1964: -0.2%
Covered Lines: 4206
Relevant Lines: 4878
💛 - Coveralls

@nyoxi
Copy link

nyoxi commented Oct 11, 2019

Please change the device path from /data/vm/disk1 to /dev/v2v-disk1 as that's the format expected by entrypoint.
https://github.com/oVirt/v2v-conversion-host/blob/cfe8e00f9d6005b6e19aac67be4c50b3d38724fe/kubevirt-conversion/entrypoint#L27

The v2v-conversion-temp is recently expected to be mounted to /var/tmp.
It is being created on default storage class.

If it's storage class defaults to volumeMode: Block (instead of so far tested Filesystem), can the conversion pod deal with it? The block device is still being added to the /var/tmp. Shouldn't we use /dev/something instead and handle that case in the conversion pod?

I do not see this happening in oVirt/v2v-conversion-host@cfe8e00 .

No, we still expect temporary PV to be mounted to /var/tmp. Is this something we need to handle too? Making filesystem on a devices is not a problem, but I wonder if the container has sufficient privileges to mount it to /var/tmp.

@mareklibra mareklibra force-pushed the bz1756328.conversionPodBlockMode branch from 66831d9 to c6c768d Compare October 14, 2019 08:47
@mareklibra
Copy link
Contributor Author

Please change the device path from /data/vm/disk1 to /dev/v2v-disk1 as that's the format expected by entrypoint.

Fixed, thanks for spotting it.

@kubevirt-bot kubevirt-bot added dco-signoff: no needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed dco-signoff: yes labels Oct 15, 2019
@mareklibra
Copy link
Contributor Author

Per offline discussion, the temporary disk is requested with the Filesystem volumeMode, no matter of the kubevirt-storage-class-defaults states for the default storage class.

Cc @nyoxi

@mareklibra mareklibra force-pushed the bz1756328.conversionPodBlockMode branch from f385def to 00a0e9b Compare October 15, 2019 12:54
@kubevirt-bot kubevirt-bot added dco-signoff: yes and removed dco-signoff: no needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 15, 2019
@mareklibra
v2v-conversion-pod: the temp disk is always of volumeMode=Filesystem
6741c0b 
To make flows in the conversion pod easier, the temporary disk is requested with
the Filesystem volumeMode no matter of the kubevirt-storage-class-defaults values.

The temporary space for the conversion pod is requested on the default storage class.
The "Filesystem" volumeMode seems to be safe, supported by all storage classes.

The target disks for the v2v stay untouched, this change affects just the temporary space.

Signed-off-by: Marek Libra <[email protected]>
@mareklibra mareklibra force-pushed the bz1756328.conversionPodBlockMode branch from 00a0e9b to 6741c0b Compare October 16, 2019 07:34
@jelkosz jelkosz merged commit 2df3806 into kubevirt:master Oct 16, 2019
This was referenced Oct 16, 2019
Merged
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
dco-signoff: yes size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mareklibra @coveralls @nyoxi @jelkosz @kubevirt-bot