Skip to content

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Sep 16, 2025

This change adds a new archive module to the Coder registry. It can be used to archive user-data from pre-defined locations and restore it as well.

Here we also explore:

  • A new method of passing arrays from Terraform to Bash
  • A new method of writing Bash scripts that minimizes the interaction with terraform interpolation
  • Extensive test-suite that not only tests that Terraform options can be selected, but also the resulting script behaviors
@mafredri mafredri force-pushed the mafredri/module-archive branch 15 times, most recently from 90f7dbd to 34eca10 Compare September 18, 2025 13:21
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I've been testing this out locally.
Nice work! Left some comments down below (none blocking).
LMK when this is ready for a more in-depth review.

@mafredri mafredri marked this pull request as ready for review September 19, 2025 14:31
@mafredri mafredri changed the title [WIP] feat(registry/coder/modules): add archive module feat(registry/coder/modules): add archive module Sep 19, 2025
@mafredri mafredri requested a review from johnstcn September 22, 2025 09:45
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Nice work! Just a couple of nits below.

Copy link
Member

@matifali matifali left a comment

Choose a reason for hiding this comment

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

This change adds a new archive module to the Coder registry. It can be used to archive user-data from pre-defined locations and restore it as well.

Where do these locations live? I see in examples that they are archived in /tmp which is not persistent in our example templates. Should we add examples on how we can archive and upload to an external storage like a s2 bucket and then restore from that?

The 2nd suggestion is to move this to coder-labs namespace.

@mafredri
Copy link
Member Author

Where do these locations live? I see in examples that they are archived in /tmp which is not persistent in our example templates. Should we add examples on how we can archive and upload to an external storage like a s2 bucket and then restore from that?

That's up to the author currently. They can store the archive in a place that's persisted or upload it. Ideally this module can be kept focused on archiving, and uploading/downloading could be handled by another module or scripted by template author.

I can add an example how upload/download could be implemented.

The 2nd suggestion is to move this to coder-labs namespace.

Didn't realize we have this, that works 👍🏻

@DevelopmentCats
Copy link
Contributor

Where do these locations live? I see in examples that they are archived in /tmp which is not persistent in our example templates. Should we add examples on how we can archive and upload to an external storage like a s2 bucket and then restore from that?

That's up to the author currently. They can store the archive in a place that's persisted or upload it. Ideally this module can be kept focused on archiving, and uploading/downloading could be handled by another module or scripted by template author.

I can add an example how upload/download could be implemented.

The 2nd suggestion is to move this to coder-labs namespace.

Didn't realize we have this, that works 👍🏻

I am pulling this to test and then we can get this merged.

@mafredri can you move this to the coder-labs namespace please?

@mafredri mafredri force-pushed the mafredri/module-archive branch from b086f29 to 2a555a9 Compare October 9, 2025 13:13
@mafredri
Copy link
Member Author

mafredri commented Oct 9, 2025

@DevelopmentCats done 👍🏻

@matifali matifali changed the title feat(registry/coder/modules): add archive module feat(registry/coder-labs/modules): add archive module Oct 9, 2025
@DevelopmentCats
Copy link
Contributor

DevelopmentCats commented Oct 14, 2025

@mafredri

I was testing this out, can you make it so that when extract_on_start is set to true, it wont fail out when there is no archive present?

It would probably be better if we just log something like: "Archive not found in backup path. (this is expected with new workspaces.)" and then continue on in the script instead of failing out as it currently does.

@mafredri mafredri force-pushed the mafredri/module-archive branch 2 times, most recently from 9e3b67c to b8acd89 Compare October 15, 2025 16:19
@mafredri
Copy link
Member Author

@DevelopmentCats I believe what you're seeing is due to a long-standing issue in coder/terraform, not this module.

This ticket has instructions on how to configure some providers to support run_on_stop: coder/coder#6174.

Whereas this ticket tracks a feature request to handle it in coder in an agnostic way: coder/coder#6175

We could improve the documentation in this module to mention that run_on_stop may not work as expected without additional configuration. Wdyt?

@DevelopmentCats
Copy link
Contributor

@DevelopmentCats I believe what you're seeing is due to a long-standing issue in coder/terraform, not this module.

This ticket has instructions on how to configure some providers to support run_on_stop: coder/coder#6174.

Whereas this ticket tracks a feature request to handle it in coder in an agnostic way: coder/coder#6175

We could improve the documentation in this module to mention that run_on_stop may not work as expected without additional configuration. Wdyt?

Yeah that's what I was referring to. My bad im sometimes bad at explaining things 😸

I think it might at least be worth having a warning GFM alert at least until run_on_stop for coder_script works as intended.

This might not happen on most workspace architectures, but when it does I imagine it would be super confusing to troubleshoot for the end user

@mafredri mafredri force-pushed the mafredri/module-archive branch from 509e284 to 7d63409 Compare October 16, 2025 14:19
@mafredri
Copy link
Member Author

@DevelopmentCats no worries. And thanks, I see you also found the bug, it was actually broken due to rename. 😅

WDYT about the warning in 7d63409 (#422)?

@DevelopmentCats
Copy link
Contributor

Looks good to me 😀

@DevelopmentCats DevelopmentCats changed the title feat(registry/coder-labs/modules): add archive module feat: add archive module Oct 16, 2025
@DevelopmentCats
Copy link
Contributor

@mafredri If we address the icon stuff we are good to merge this

@mafredri mafredri force-pushed the mafredri/module-archive branch from 583829f to 4029d9f Compare October 17, 2025 09:00
@mafredri
Copy link
Member Author

@DevelopmentCats I copied over folder.svg from coder/coder and referenced it. Perhaps one related to archiving/compression would be a bit better but I think it'll do for now. Thanks for the hint.

@mafredri
Copy link
Member Author

It'd be nice to add an icon linter, we could fairly easily verify that the referenced files exist in registry and/or coder.

@DevelopmentCats
Copy link
Contributor

It'd be nice to add an icon linter, we could fairly easily verify that the referenced files exist in registry and/or coder.

100% agree with this

@DevelopmentCats DevelopmentCats merged commit e34320c into main Oct 17, 2025
4 checks passed
@DevelopmentCats DevelopmentCats deleted the mafredri/module-archive branch October 17, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants