Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(xo-web/import/esxi): import VM from ESXi #6663

Merged
merged 28 commits into from
Feb 27, 2023
Merged

feat(xo-web/import/esxi): import VM from ESXi #6663

merged 28 commits into from
Feb 27, 2023

Conversation

Rajaa-BARHTAOUI
Copy link
Contributor

@Rajaa-BARHTAOUI Rajaa-BARHTAOUI commented Feb 8, 2023

[
  {
    id: '1',
    nameLabel: 'test_linux',
    memory: 2147483648,
    nCpus: 1,
    guestToolsInstalled: false,
    firmware: 'bios',
    powerState: 'poweredOff',
    storage: {
      used: 216761,
      free: 2318061568,
    },
  },
  {
    id: '2',
    nameLabel: 'test_small',
    memory: 2147483648,
    nCpus: 1,
    guestToolsInstalled: false,
    firmware: 'bios',
    powerState: 'poweredOff',
    storage: {
      used: 10696,
      free: 2586497520,
    },
  },
]
  • Params provided to vm.importFromEsxi
 {
    host: '192.168.2.20',
    network: '6b309876-9b61-6feb',
    password: 'password',
    sr: '86a9757d-9c05-9fe0-e79a',
    sslVerify: false,
    user: 'user',
    vm: '1',
    },
  }
  

Screenshots

Capture d’écran du 2023-02-16 21-12-04

Capture d’écran du 2023-02-16 21-12-45

Capture d’écran du 2023-02-21 16-08-11

Capture d’écran du 2023-02-21 16-13-15

Check list

Check if done, if not relevant leave unchecked.

  • PR reference the relevant issue (e.g. Fixes #007 or See xoa-support#42)
  • if UI changes, a screenshot has been added to the PR
  • documentation updated
  • CHANGELOG.unreleased.md:
    • enhancement/bug fix entry added
    • list of packages to release updated (${name} v${new version})
  • I have tested added/updated features (and impacted code)

Process

  1. create a PR as soon as possible
  2. mark it as WiP: (Work in Progress) if not ready to be merged
  3. when you want a review, add a reviewer (and only one)
  4. if necessary, update your PR, and re- add a reviewer

From the Four Agreements:

  1. Be impeccable with your word.
  2. Don't take anything personally.
  3. Don't make assumptions.
  4. Always do your best.

@fbeauchamp
Copy link
Collaborator

Tests done :

  • can't import from ova ( when dropping an ova it does not show the network/ disk form )
  • can't import from xva import button stay disabled when dropping a xva
  • can't select ova format when importing from url
  • can't unselect ssl certificate page when importing to esxi ( thus, I am not able to test esxi import since our server need to uncheck this)

@Rajaa-BARHTAOUI Rajaa-BARHTAOUI removed the request for review from fbeauchamp February 14, 2023 09:33
@olivierlambert
Copy link
Member

I would prefer a dedicated page, as explained in the original issue (Import/From VMware), allowing multi VMs import, which is a special process we don't have in the "Import/VM" view.

@Rajaa-BARHTAOUI Rajaa-BARHTAOUI requested review from fbeauchamp and removed request for fbeauchamp February 14, 2023 09:49
@Rajaa-BARHTAOUI
Copy link
Contributor Author

Rajaa-BARHTAOUI commented Feb 16, 2023

Tests done :

* can't import from ova ( when dropping an ova it does not show the network/ disk form )

* can't import from xva import button stay disabled when dropping a xva

* can't select ova format when importing from url

* can't unselect ssl certificate page when importing to esxi ( thus, I am not able to test esxi import since our server need to uncheck this)
  • Fixed
  • Fixed
  • It's already the case: URL import is only for XVA files
  • Fixed

@Rajaa-BARHTAOUI
Copy link
Contributor Author

I would prefer a dedicated page, as explained in the original issue (Import/From VMware), allowing multi VMs import, which is a special process we don't have in the "Import/VM" view.

  • As a first step, we decided to allow the import of one VM.

@fbeauchamp
Copy link
Collaborator

fbeauchamp commented Feb 17, 2023

test done : rebase from master ( since there is fix on import, merge from branch #6662 )

  • import ova /xva : import ok, but got error :
    image
  • import from esxi :
    • connect ok
    • list VMs show a list without any label
    • the VM to import don't have any detail except memory
    • clicking on the import give an invalid parameter message
      2023-02-17T10_59_59.772Z - XO.log

@fbeauchamp
Copy link
Collaborator

Test done

test Protocol

  • pulled this branch
  • rebase master onto it, since there is fixes on ova /vmdk handling
  • merged the backend branch into it to get new response format from esxi.connect api method

import XVA

  • the import button does not stay disabled during the import
  • a succes alert should be shown at the end
  • user should be redirected to imported VM when done
  • Not tested : the button should turn orange on import error
  • import seems to have worked

import Ova

  • the import button does not stay disabled with text importing during import
  • user should be redirected to imported VM when done
  • a succes alert should be shown at the end
  • Not tested : the button should turn orange on import error
  • import seems to have worked

import from url

  • not tested

import from esxi

  • connect ok
  • connect with improper parameters : show error ok
  • vm listing ok
  • network should have a default value ( as during the ova import )
  • additionnal fields : not handled by backend or now
  • the import button does not stay disabled with text importing during import
  • a succes alert should be shown at the end
  • user should be redirected to imported VM when done (when importing 1 VM )

@Rajaa-BARHTAOUI Rajaa-BARHTAOUI force-pushed the importEsxiUI branch 3 times, most recently from 65e6bdc to 9918303 Compare February 21, 2023 16:10
@fbeauchamp
Copy link
Collaborator

Test

Protocol

branch are up to date and includes necessary backend method

ova /xva import

  • got a translation problem :
    image
  • button : OK
  • import : ok
  • success toast ok
  • redirect after importing : Not ok

from esxi

  • bad connect data : error message is ok
  • connect with the right data : ok
  • Vm listing : ok
  • importing fail with
    image
    Reselecting the network and it works
  • importing : ok
  • button : ok

@fbeauchamp
Copy link
Collaborator

Test

protocol

only need to pull the branch

xva / ova import : ok

import esxi : still running , but no error on start

Copy link
Collaborator

@fbeauchamp fbeauchamp left a comment

Choose a reason for hiding this comment

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

functionnal tests are ok for me

Copy link
Member

@pdonias pdonias left a comment

Choose a reason for hiding this comment

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

LGTM but I don't know how to test it. @Rajaa-BARHTAOUI or @fbeauchamp, can you do a last end-to-end test after the latest changes?

@Rajaa-BARHTAOUI
Copy link
Contributor Author

LGTM but I don't know how to test it. @Rajaa-BARHTAOUI or @fbeauchamp, can you do a last end-to-end test after the latest changes?

It has been tested after the latest changes, and everything is still running.

@pdonias
Copy link
Member

pdonias commented Feb 24, 2023

Ok, then this PR is ready to merge after #6662.

@pdonias pdonias changed the title feat(xo-web/import/esxi): import VM from esxi feat(xo-web/import/esxi): import VM from ESXi Feb 27, 2023
@pdonias pdonias merged commit e6c95a0 into master Feb 27, 2023
@pdonias pdonias deleted the importEsxiUI branch February 27, 2023 09:12
MathieuRA pushed a commit that referenced this pull request Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants