Skip to content

Commit

Permalink
fix an issue where machine catalog creation fails when using both cit…
Browse files Browse the repository at this point in the history
…rix prepared image and azure ephemeral disk
  • Loading branch information
yashwanthjammi committed Feb 27, 2025
1 parent 5df2aa9 commit d4bb8ca
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 169 deletions.
4 changes: 3 additions & 1 deletion docs/resources/machine_catalog.md
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,9 @@ Optional:
- `machine_profile` (Attributes) The name of the virtual machine or template spec that will be used to identify the default value for the tags, virtual machine size, boot diagnostics, host cache property of OS disk, accelerated networking and availability zone.<br />Required when provisioning_type is set to PVSStreaming or when identity_type is set to `AzureAD` (see [below for nested schema](#nestedatt--provisioning_scheme--azure_machine_config--machine_profile))
- `master_image_note` (String) The note for the master image.
- `prepared_image` (Attributes) Specifying the prepared master image to be used for machine catalog. (see [below for nested schema](#nestedatt--provisioning_scheme--azure_machine_config--prepared_image))
- `use_azure_compute_gallery` (Attributes) Use this to place prepared image in Azure Compute Gallery. Required when `storage_type = Azure_Ephemeral_OS_Disk`. (see [below for nested schema](#nestedatt--provisioning_scheme--azure_machine_config--use_azure_compute_gallery))
- `use_azure_compute_gallery` (Attributes) Use this to place prepared image in Azure Compute Gallery. Required when `storage_type = Azure_Ephemeral_OS_Disk`.

~> **Please Note** `use_azure_compute_gallery` cannot be specified when the prepared image is using a shared image gallery. The machine catalog will inherit the azure compute gallery settings of the prepared image. (see [below for nested schema](#nestedatt--provisioning_scheme--azure_machine_config--use_azure_compute_gallery))
- `use_managed_disks` (Boolean) Indicate whether to use Azure managed disks for the provisioned virtual machine.
- `vda_resource_group` (String) Designated resource group where the VDA VMs will be located on Azure.
- `writeback_cache` (Attributes) Write-back Cache config. Leave this empty to disable Write-back Cache. Write-back Cache requires Machine image with Write-back Cache plugin installed. (see [below for nested schema](#nestedatt--provisioning_scheme--azure_machine_config--writeback_cache))
Expand Down
4 changes: 2 additions & 2 deletions internal/daas/image_definition/image_version_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (r *ImageVersionResource) Create(ctx context.Context, req resource.CreateRe
}

if !plan.AzureImageSpecs.IsNull() {
azureImageSpecs := util.ObjectValueToTypedObject[AzureImageSpecsModel](ctx, &resp.Diagnostics, plan.AzureImageSpecs)
azureImageSpecs := util.ObjectValueToTypedObject[util.AzureImageSpecsModel](ctx, &resp.Diagnostics, plan.AzureImageSpecs)
sharedSubscription := azureImageSpecs.SharedSubscription.ValueString()
resourceGroup := azureImageSpecs.ResourceGroup.ValueString()
masterImage := azureImageSpecs.MasterImage.ValueString()
Expand Down Expand Up @@ -495,7 +495,7 @@ func (r *ImageVersionResource) ModifyPlan(ctx context.Context, req resource.Modi
)
return
}
azureImageSpecs := util.ObjectValueToTypedObject[AzureImageSpecsModel](ctx, &resp.Diagnostics, plan.AzureImageSpecs)
azureImageSpecs := util.ObjectValueToTypedObject[util.AzureImageSpecsModel](ctx, &resp.Diagnostics, plan.AzureImageSpecs)
// Validate image version machine profile usage consistency within the image definition
imageVersionsInDefinition, err := getImageVersions(ctx, &resp.Diagnostics, r.client, plan.ImageDefinition.ValueString())
if err != nil {
Expand Down
124 changes: 3 additions & 121 deletions internal/daas/image_definition/image_version_resource_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ import (
"github.com/citrix/terraform-provider-citrix/internal/util"
"github.com/hashicorp/terraform-plugin-framework-validators/int32validator"
"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/objectvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int32planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier"
Expand All @@ -27,95 +25,6 @@ import (
"github.com/hashicorp/terraform-plugin-framework/types"
)

type AzureImageSpecsModel struct {
// Required Attributes
ServiceOffering types.String `tfsdk:"service_offering"`
LicenseType types.String `tfsdk:"license_type"`
StorageType types.String `tfsdk:"storage_type"`

// Optional Attributes
MachineProfile types.Object `tfsdk:"machine_profile"`
DiskEncryptionSet types.Object `tfsdk:"disk_encryption_set"`

// Master Image Attributes
ResourceGroup types.String `tfsdk:"resource_group"`
SharedSubscription types.String `tfsdk:"shared_subscription"`
MasterImage types.String `tfsdk:"master_image"`
GalleryImage types.Object `tfsdk:"gallery_image"`
}

func (AzureImageSpecsModel) GetSchema() schema.SingleNestedAttribute {
galleryImageSchema := util.GalleryImageModel{}.GetSchema()
galleryImageSchema.Validators = []validator.Object{
objectvalidator.AlsoRequires(path.Expressions{
path.MatchRelative().AtParent().AtName("resource_group"),
}...),
}
return schema.SingleNestedAttribute{
Description: "Image configuration for Azure image version.",
Optional: true,
Attributes: map[string]schema.Attribute{
"service_offering": schema.StringAttribute{
Description: "The Azure VM Sku to use when creating machines.",
Required: true,
},
"license_type": schema.StringAttribute{
Description: "Windows license type used to provision virtual machines in Azure at the base compute rate. License types include: `Windows_Client` and `Windows_Server`.",
Optional: true,
Validators: []validator.String{
stringvalidator.OneOf(
util.WindowsClientLicenseType,
util.WindowsServerLicenseType,
),
},
},
"storage_type": schema.StringAttribute{
Description: "Storage account type used for provisioned virtual machine disks on Azure. Storage types include: `Standard_LRS`, `StandardSSD_LRS` and `Premium_LRS`.",
Required: true,
Validators: []validator.String{
stringvalidator.OneOf(
util.StandardLRS,
util.StandardSSDLRS,
util.Premium_LRS,
),
},
},
"machine_profile": util.AzureMachineProfileModel{}.GetSchema(),
"disk_encryption_set": util.AzureDiskEncryptionSetModel{}.GetSchema(),
"resource_group": schema.StringAttribute{
Description: "The Azure Resource Group where the managed disk / snapshot for creating machines is located.",
Required: true,
},
"shared_subscription": schema.StringAttribute{
Description: "The Azure Subscription ID where the managed disk / snapshot for creating machines is located. Only required if the image is not in the same subscription of the hypervisor.",
Optional: true,
Validators: []validator.String{
stringvalidator.LengthAtLeast(1),
},
},
"master_image": schema.StringAttribute{
Description: "The name of the virtual machine snapshot or VM template that will be used. This identifies the hard disk to be used and the default values for the memory and processors. Omit this field if you want to use gallery_image.",
Optional: true,
Validators: []validator.String{
stringvalidator.LengthAtLeast(1),
stringvalidator.ExactlyOneOf(path.MatchRelative().AtParent().AtName("gallery_image")),
},
},
"gallery_image": galleryImageSchema,
},
PlanModifiers: []planmodifier.Object{
objectplanmodifier.RequiresReplace(),
},
Validators: []validator.Object{
objectvalidator.ExactlyOneOf(path.MatchRelative().AtParent().AtName("vsphere_image_specs")),
},
}
}

func (AzureImageSpecsModel) GetAttributes() map[string]schema.Attribute {
return AzureImageSpecsModel{}.GetSchema().Attributes
}

type VsphereImageSpecsModel struct {
MasterImageVm types.String `tfsdk:"master_image_vm"`
ImageSnapshot types.String `tfsdk:"image_snapshot"`
Expand Down Expand Up @@ -273,7 +182,7 @@ func (ImageVersionModel) GetSchema() schema.Schema {
Computed: true,
Default: stringdefault.StaticString(""),
},
"azure_image_specs": AzureImageSpecsModel{}.GetSchema(),
"azure_image_specs": util.AzureImageSpecsModel{}.GetSchema(),
"vsphere_image_specs": VsphereImageSpecsModel{}.GetSchema(),
"session_support": schema.StringAttribute{
Description: "Session support for the image version.",
Expand Down Expand Up @@ -320,7 +229,7 @@ func (r ImageVersionModel) RefreshPropertyValues(ctx context.Context, diagnostic

switch imageContext.GetPluginFactoryName() {
case util.AZURERM_FACTORY_NAME:
azureImageSpecs := util.ObjectValueToTypedObject[AzureImageSpecsModel](ctx, diagnostics, r.AzureImageSpecs)
azureImageSpecs := util.ObjectValueToTypedObject[util.AzureImageSpecsModel](ctx, diagnostics, r.AzureImageSpecs)

azureImageSpecs.ServiceOffering = parseAzureImageVersionServiceOffering(imageScheme.GetServiceOffering())

Expand All @@ -337,7 +246,7 @@ func (r ImageVersionModel) RefreshPropertyValues(ctx context.Context, diagnostic
azureImageSpecs.MachineProfile = updatedMachineProfile
}

azureImageSpecs = ParseMasterImageToAzureImageModel(ctx, diagnostics, azureImageSpecs, masterImage)
azureImageSpecs = util.ParseMasterImageToAzureImageModel(ctx, diagnostics, azureImageSpecs, masterImage)
r.AzureImageSpecs = util.TypedObjectToObjectValue(ctx, diagnostics, azureImageSpecs)
case util.VMWARE_FACTORY_NAME:
imageScheme := imageContext.GetImageScheme()
Expand Down Expand Up @@ -373,33 +282,6 @@ func (r ImageVersionModel) RefreshPropertyValues(ctx context.Context, diagnostic
return r
}

func ParseMasterImageToAzureImageModel(ctx context.Context, diagnostics *diag.Diagnostics, azureImageSpecs AzureImageSpecsModel, masterImage citrixorchestration.HypervisorResourceRefResponseModel) AzureImageSpecsModel {
masterImageXdPath := masterImage.GetXDPath()
masterImageSegments := strings.Split(masterImageXdPath, "\\")
masterImageLastIndex := len(masterImageSegments)
masterImageResourceTag := strings.Split(masterImageSegments[masterImageLastIndex-1], ".")
masterImageResourceType := masterImageResourceTag[len(masterImageResourceTag)-1]
if strings.EqualFold(masterImageResourceType, util.ImageVersionResourceType) {
azureImageSpecs.GalleryImage,
azureImageSpecs.ResourceGroup,
azureImageSpecs.SharedSubscription =
util.ParseMasterImageToUpdateGalleryImageModel(ctx, diagnostics, azureImageSpecs.GalleryImage, masterImage, masterImageSegments, masterImageLastIndex)

// Clear other master image details
azureImageSpecs.MasterImage = types.StringNull()
} else {
// Snapshot or Managed Disk
azureImageSpecs.MasterImage,
azureImageSpecs.ResourceGroup,
azureImageSpecs.SharedSubscription,
azureImageSpecs.GalleryImage,
_,
_ =
util.ParseMasterImageToUpdateAzureImageSpecs(ctx, diagnostics, masterImageResourceType, masterImage, masterImageSegments, masterImageLastIndex)
}
return azureImageSpecs
}

func parseVsphereImageXdPath(masterImageXdPath string) (string, string) {
vmIndex := strings.Index(masterImageXdPath, ".vm")
if vmIndex == -1 {
Expand Down
30 changes: 23 additions & 7 deletions internal/daas/machine_catalog/machine_catalog_common_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,22 +499,23 @@ func validateInUseMachineAccounts(ctx context.Context, diagnostics *diag.Diagnos
return nil
}

func validateImageVersion(ctx context.Context, diagnostics *diag.Diagnostics, client *citrixdaasclient.CitrixDaasClient, plan MachineCatalogResourceModel, preparedImageConfig PreparedImageConfigModel, machineConfigAttributeName string) (citrixorchestration.ImageSchemeResponseModel, citrixorchestration.ImageVersionSpecResponseModel, error) {
func validateImageVersion(ctx context.Context, diagnostics *diag.Diagnostics, client *citrixdaasclient.CitrixDaasClient, plan MachineCatalogResourceModel, preparedImageConfig PreparedImageConfigModel, machineConfigAttributeName string) (*citrixorchestration.ImageDefinitionResponseModel, citrixorchestration.ImageSchemeResponseModel, citrixorchestration.ImageVersionSpecResponseModel, error) {
isPreparedImageSupported := util.CheckProductVersion(client, diagnostics, 121, 118, 7, 41, "Error using Prepared Image in citrix_machine_catalog resource", "Prepared Image")
var imageScheme citrixorchestration.ImageSchemeResponseModel
var imageSpecs citrixorchestration.ImageVersionSpecResponseModel
var imageDefinition *citrixorchestration.ImageDefinitionResponseModel
if !isPreparedImageSupported {
err := fmt.Errorf("Prepared Image is not supported in this version of Citrix Virtual Apps and Desktops service.")
return imageScheme, imageSpecs, err
return nil, imageScheme, imageSpecs, err
}
imageDefinition, err := image_definition.GetImageDefinition(ctx, client, diagnostics, preparedImageConfig.ImageDefinition.ValueString())
if err != nil {
return imageScheme, imageSpecs, err
return imageDefinition, imageScheme, imageSpecs, err
}

imageVersion, err := image_definition.GetImageVersion(ctx, client, diagnostics, imageDefinition.GetId(), preparedImageConfig.ImageVersion.ValueString())
if err != nil {
return imageScheme, imageSpecs, err
return imageDefinition, imageScheme, imageSpecs, err
}

imageVersionStatus := imageVersion.GetImageVersionStatus()
Expand All @@ -524,7 +525,7 @@ func validateImageVersion(ctx context.Context, diagnostics *diag.Diagnostics, cl
"Error validating azure_machine_config",
err.Error(),
)
return imageScheme, imageSpecs, err
return imageDefinition, imageScheme, imageSpecs, err
}

imageContextConfigured := false
Expand All @@ -549,11 +550,12 @@ func validateImageVersion(ctx context.Context, diagnostics *diag.Diagnostics, cl
fmt.Sprintf("Error validating `%s`", machineConfigAttributeName),
err.Error(),
)
return imageScheme, imageSpecs, err
return imageDefinition, imageScheme, imageSpecs, err
}
}
}
return imageScheme, imageSpecs, nil

return imageDefinition, imageScheme, imageSpecs, nil
}

func getMachineAccountDeleteOptionValue(v string) citrixorchestration.MachineAccountDeleteOption {
Expand All @@ -565,3 +567,17 @@ func getMachineAccountDeleteOptionValue(v string) citrixorchestration.MachineAcc

return *machineAccountDeleteOption
}

func IsAzureImageDefinitionUsingSharedImageGallery(imageDefinitionResp *citrixorchestration.ImageDefinitionResponseModel) bool {
preparedImageUseSharedGallery := false
imgDefinitionConn := imageDefinitionResp.GetHypervisorConnections()
if len(imgDefinitionConn) > 0 {
customProperties := imgDefinitionConn[0].GetCustomProperties()
for _, property := range customProperties {
if property.GetName() == "UseSharedImageGallery" {
preparedImageUseSharedGallery, _ = strconv.ParseBool(property.GetValue())
}
}
}
return preparedImageUseSharedGallery
}
Loading

0 comments on commit d4bb8ca

Please sign in to comment.