Fix deletion of Guacamole VM in stopped state

[marrobi]

Draft - what think about this approach with a script to ensure child resources are deleted? Need to do other VMs.

Add a configuration option to Guacamole VM templates to skip shutdown and force deletion, resolving the issue where VMs in a stopped state cannot be deleted. Update relevant Terraform and Porter files to implement this change and increment version numbers accordingly.

Fixes #4135

[marrobi]

@tamirkamara @jonnyry attempt number 2, thoughts on this approach?

[marrobi]

@dram1964 fancy testing this one?

[dram1964]

Hi @marrobi, I did test the original fix, but didn't realise that this left the os_disk deployed. Happy to test out the new code, but I was wondering whether this is the best approach.
Given that the virtual_machine {skip_shutdown_and_force_delete = true } feature is designed to preserve sub-resources of the VM, the additional script is a 'fix for the fix'.
Would it be better to add an az vm start operation to the 'delete' step, since the 'delete' operation appears to expect the vm to be started?

[marrobi]

Hi @marrobi, I did test the original fix, but didn't realise that this left the os_disk deployed. Happy to test out the new code, but I was wondering whether this is the best approach. Given that the virtual_machine {skip_shutdown_and_force_delete = true } feature is designed to preserve sub-resources of the VM, the additional script is a 'fix for the fix'. Would it be better to add an az vm start operation to the 'delete' step, since the 'delete' operation appears to expect the vm to be started?

Agree, that's another option, just worry about turning a VM on that has been intentionally stopped. Imagine it has a start-up script that does stuff, or you turn it off as it has some malware. I'd prefer to just get rid of it.

[tamirkamara]

Wouldn't it be easier to use azure cli to delete the VM if it's not on and then let TF do its thing without any other modification?
We already have the VM resource ID available so just need to query if it exists in the right state and delete.
p.s. there seem to be a PR on AzureRM for this. So probably should add a comment to any fix here that can be removed when will move to version 4.14 (maybe?)

[marrobi]

Ah, hadn't seen that - making a note to follow to hashicorp/terraform-provider-azurerm#28412

I used this approach as thought it was generic and could be reused for other services where we get lingering resources. Providing they are tagged they will get deleted.

If it's going to be reverted, don't mind, as long as it's fixed :)

[dram1964]

@dram1964 fancy testing this one?

Hi @marrobi - sorry I'm having trouble testing this. I've copied your changes into my repo, but can't build the linuxdsvm image (make user_resource_bundle BUNDLE=guacamole-azure-linuxvm WORKSPACE_SERVICE=guacamole).

The problem seems to be caused by this line in the Dockerfile.tmpl:

COPY --from=scripts --link . ${BUNDLE_DIR}/scripts/

And the error is:

failed to resolve source metadata for docker.io/library/scripts:latest

I have added the porter-build-context.env and created the templates\scripts\delete_azure_resources.sh - but I don't think the scripts env var is actually being set.

I guess this is probably because I'm on an earlier version of the TRE (v0.19.1), with older versions of linuxdsvm (1.0.3)?

[marrobi]

@dram1964 fancy testing this one?

Hi @marrobi - sorry I'm having trouble testing this. I've copied your changes into my repo, but can't build the linuxdsvm image (make user_resource_bundle BUNDLE=guacamole-azure-linuxvm WORKSPACE_SERVICE=guacamole).

The problem seems to be caused by this line in the Dockerfile.tmpl:

COPY --from=scripts --link . ${BUNDLE_DIR}/scripts/

And the error is:

failed to resolve source metadata for docker.io/library/scripts:latest

I have added the porter-build-context.env and created the templates\scripts\delete_azure_resources.sh - but I don't think the scripts env var is actually being set.

I guess this is probably because I'm on an earlier version of the TRE (v0.19.1), with older versions of linuxdsvm (1.0.3)?

@dram1964 ah, yes, could be the new porter support for build contexts has only recently been added so need to be on the latest version of porter. The latest is in the main branch devcontainer.

[jonnyry]

@marrobi which resources are left lingering after the delete has applied? Is it just the osdisk?

If so delete_os_disk_on_deletion should do the trick according to this comment:

features {
        virtual_machine {
          skip_shutdown_and_force_delete = true
          delete_os_disk_on_deletion = true
        }
      }