Skip to content

feat(instanceV2) - Add imageName and ImageNameRef to the blockDevice … - #177

Merged
lappesj merged 1 commit into
crossplane-contrib:mainfrom
JRaver:feature/add-image-name-ref
Oct 27, 2026
Merged

lappesj merged 1 commit into
crossplane-contrib:mainfrom
JRaver:feature/add-image-name-ref

Conversation

@JRaver

@JRaver JRaver commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

PR: Add imageName to blockDevice for InstanceV2

Description of your changes

Adds imageName / imageNameRef / imageNameSelector support to blockDevice[] entries of InstanceV2 (both cluster and namespaced scope).

Background

The upstream Terraform provider resolves image_name → image UUID only for the top-level image_name field (ephemeral-disk boot). For boot-from-volume (block_device[].uuid, source_type: image, destination_type: volume) the provider requires a raw image UUID. This made it impossible to write portable manifests across clusters where the same image has different UUIDs.

What this PR does

New field blockDevice[].imageName

  • Resolved to an image UUID at create-time via ImageV2Client + imagesutils.IDFromName (Glance API), without any changes to the upstream Terraform provider.
  • Mutually exclusive with uuid; only valid for source_type: image.
  • ForceNew: true — changing the name is detected as drift and blocks the update (refuse to update … requires replacing it), consistent with the existing uuid behaviour.

Drift detection via ReadContext wrapper

  • After each reconcile Read, the provider looks up the current name of the boot-volume image (by UUID) in Glance and writes it back to the Terraform state as image_name.
  • On the next reconcile, CustomizeDiff compares the config name with the observed name. A change produces a ForceNew diff that upjet refuses to apply, surfacing a clear error.
  • Lookup failures (deleted / inaccessible image) are silently ignored so they never cause a false drift.

imageNameRef / imageNameSelector — optional; resolves the name from a ImageV2 managed resource using ExtractParamPath("name", false). Works with both fully-managed and managementPolicies: [Observe] images (for the latter, set spec.forProvider.name explicitly).

Late-initialize guard — BlockDevice.ImageName is added to LateInitializer.IgnoredFields so it is never overwritten by the observed UUID during late-init.

Usage examples

Direct name (no ImageV2 object required):

apiVersion: compute.openstack.crossplane.io/v1alpha1
kind: InstanceV2
metadata:
  name: my-vm
spec:
  forProvider:
    blockDevice:
    - bootIndex: 0
      sourceType: image
      destinationType: volume
      imageName: Ubuntu_2404_openstack
      volumeSize: 30
      volumeType: __DEFAULT__
      deleteOnTermination: true
    flavorName: vm.c4r8

With reference to an observe-only ImageV2:

# On the cluster that owns the image:
apiVersion: images.openstack.crossplane.io/v1alpha1
kind: ImageV2
metadata:
  annotations:
    crossplane.io/external-name: 36ad9881-eb9c-47f3-9d82-be9327998288
  name: ubuntu-2404
spec:
  forProvider:
    name: Ubuntu_2404_openstack   # required for imageNameRef resolution
  managementPolicies: [Observe]
  providerConfigRef:
    name: default
---
apiVersion: compute.openstack.crossplane.io/v1alpha1
kind: InstanceV2
metadata:
  name: my-vm
spec:
  forProvider:
    blockDevice:
    - bootIndex: 0
      sourceType: image
      destinationType: volume
      imageNameRef:
        name: ubuntu-2404
      volumeSize: 30
      deleteOnTermination: true

I have

  • Read and followed Crossplane's contribution process.
  • Run make submodules; make reviewable test to ensure this PR is ready for review.

How has this code been tested

  • go test ./config/common/blockdevice/... — unit tests covering:
    • schema extension (image_name field present, DiffSuppressFunc set on uuid)
    • CreateContext wrapper: name resolved to UUID before upstream create
    • validation errors: uuid + imageName conflict, wrong source_type, image not found, multiple images with same name
    • ReadContext wrapper: image name populated from Glance after read
    • lookup failure during read does not fail reconciliation and preserves previous state
    • diff table: new resource, same name (no diff), changed name (ForceNew diff), no observed name (no diff), uuid-only config, changed uuid
  • make generate — verified generated types (ImageName, ImageNameRef, ImageNameSelector in blockDevice for both cluster and namespaced scope), resolvers, deepcopy, CRDs
  • make reviewable test — 0 issues, all unit tests pass
  • Live cluster (OpenStack):
    • InstanceV2 with imageName: Ubuntu_2404_openstack created successfully; status.atProvider.blockDevice[0].imageName shows the resolved name after first reconcile
    • Changing imageName to a different image name → Synced=False, error refuse to update … cannot change the value of the argument "block_device.0.image_name" — server untouched
    • imageNameRef pointing to an observe-only ImageV2 with forProvider.name set — resolves correctly
    • Non-existent image name → create-time error from Glance before any Nova call

@JRaver
JRaver requested a review from Avatos as a code owner October 15, 2026 09:11
@JRaver

JRaver commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@Avatos Please check the PR when you have time, thanks

@JRaver
JRaver force-pushed the feature/add-image-name-ref branch from 3a2662d to 2297c8a Compare October 23, 2026 14:55
@JRaver
JRaver requested a review from lappesj as a code owner October 23, 2026 14:55
@JRaver

JRaver commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@lappesj Hello, maybe you can look at the my PR please.

@lappesj

lappesj commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Hey @JRaver,

I found an issue when blockDevice.imageName is set in initProvider and late initialization is enabled. After creation, late initialization copies the resolved image UUID into forProvider. If the instance is later deleted, Crossplane tries to recreate it using both the original image name and that UUID. Creation then fails with:
image_name and uuid are mutually exclusive

I reproduced this for both cluster-scoped and namespaced resources.

Could we skip late-initializing blockDevice when it’s already set in initProvider? Upjet’s ConditionalIgnoredFields looks suitable for this. A regression test covering recreation would help too.

@JRaver
JRaver force-pushed the feature/add-image-name-ref branch from 2297c8a to 7af7246 Compare October 26, 2026 08:16
@JRaver

JRaver commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Hello, @lappesj. Thank you for catching this!

The issue was a late-initialization race: after creation, upjet copied the resolved image UUID from the observed Terraform state into spec.forProvider.blockDevice[].uuid. On recreation, the merged params then contained both imageName (from initProvider) and that late-initialized uuid, triggering our "mutually exclusive" validation.

Fixed in two layers:

  1. LateInitializer.IgnoredFields — added block_device.uuid alongside the existing block_device.image_name. This prevents upjet from ever copying the observed UUID into forProvider, keeping it initProvider-exclusive (which upjet correctly ignores on diff).

  2. resolveImageNames fallback — instead of returning an error when both fields are present, imageName now silently wins and the stale uuid is cleared. This handles any edge cases where both values end up in the merged params regardless of the late-init guard.

A regression test (TestCreateImageNameWinsOverLateInitUUID) was added to cover exactly this scenario.

Please check it now, when you will have time, thanks

@JRaver
JRaver force-pushed the feature/add-image-name-ref branch from 7af7246 to 76e9401 Compare October 26, 2026 08:19
@lappesj

lappesj commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the update, @JRaver . I tested commit 76e9401. The original recreation test now passes for cluster-scoped and namespaced resources.

I found two remaining problems.

1. The UUID exclusion also affects devices without imageName

The new late-initialization filter excludes block_device.uuid for all block devices.

For an existing UUID-based device, I tested these steps:

  1. Remove spec.forProvider.blockDevice.
  2. Let late initialization restore the list from the observed state.
  3. Check the next Terraform diff.

Late initialization restores the other device fields, but not the UUID. The next diff requires instance replacement because the desired UUID is absent. Upjet refuses this update.

I reproduced this for volume and snapshot devices in both scopes. The same tests pass with the previous late-initialization code.

Could you limit the UUID exclusion to the image-name case?

2. The fallback does not correct the stored UUID

The fallback replaces the UUID in the creation data. It does not change the UUID stored in spec.forProvider.

If imageName resolves to a different UUID, creation succeeds. However, the next reconciliation still requests the stored UUID. This difference requires instance replacement, which Upjet refuses.

This can affect an existing resource if an image is replaced and the new image has the same name.

Could you handle the stored UUID conflict and extend the regression test to check the next diff after creation? I reproduced this problem in both scopes.

…sections

Signed-off-by: Yevhen Artemenko <eartemenko@playtika.com>
@JRaver
JRaver force-pushed the feature/add-image-name-ref branch from 76e9401 to 903b16a Compare October 27, 2026 10:35

@lappesj lappesj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed and tested 903b16a. Both previously reported late-initialization/recreation issues are fixed in cluster and namespaced scope. Full Go tests, go vet, provider/generator builds, and focused race/regression tests passed. CI coverage for config/common/blockdevice will be handled separately.

@lappesj
lappesj merged commit c1981cf into crossplane-contrib:main Oct 27, 2026
8 checks passed
@JRaver

JRaver commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks you @lappesj for your help). Waiting for the release )

@lappesj

lappesj commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks for contributing @JRaver . It's released as https://github.com/crossplane-contrib/provider-openstack/releases/tag/v0.12.0

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.

2 participants