theforeman / foreman_azure_rm

Adds Azure Resource Manager as a compute resource for The Foreman
GNU General Public License v3.0
9 stars 24 forks source link

Fixes #29819 - Support byos images on azurerm #88

Closed apuntamb closed 4 years ago

apuntamb commented 4 years ago

BYOS images on azure are a different type of marketplace images. They just need an added parameter from PurchasePlan class of the sdk. This PR provisions the host using rhel byos image if the image terms have been accepted. Refer the MSFT doc to accept image terms. Currently, I have to test if with this change the already supported images still function correctly or not hence, adding WIP label. Will remove the label once verified that it works smoothly for every single image category.

vijay8451 commented 4 years ago

@ShimShtein sure will give a try once have this enable over my Azure cloud account .

apuntamb commented 4 years ago

Tested VM creation as per Vijay's directions as he couldn't get the byos images activated for his az account. VM creation works fine. Let's see if @vijay8451 has some results for image creation testing.

vijay8451 commented 4 years ago

Test Results: Works:

Found a minor issue:

i.e. you could create two images using below URNs and the only diff in them is R/r in RedHat

marketplace://RedHat:rhel-byos:rhel-lvm81:8.1.2019123006
marketplace://redHat:rhel-byos:rhel-lvm81:8.1.2019123006

@ShimShtein @apuntamb thanks ACK from my end .

apuntamb commented 4 years ago

Test Results: Works:

* Able to create/edit/delete BYOS image using `marketplace://` prefix

* Able to create/edit/delete marketplace image using `marketplace://` prefix

* Able to create/edit/delete shared gallery image using `gallery://` prefix

* Able to create/edit/delete custom image using `custom://` prefix

Found a minor issue:

* Able to create duplicate images using same URN (by just changing the case)

i.e. you could create two images using below URNs and the only diff in them is R/r in RedHat

marketplace://RedHat:rhel-byos:rhel-lvm81:8.1.2019123006
marketplace://redHat:rhel-byos:rhel-lvm81:8.1.2019123006

Good catch @vijay8451! I have fixed this too in my latest commit.

@ShimShtein @apuntamb thanks ACK from my end .

@ShimShtein I have added case sensitive validation in the engine file for this. Also, mentioned byos-images in the Image create form.

ShimShtein commented 4 years ago

Merged, thanks @apuntamb!