Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: import value from legacy variable to build parameter#6556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mtojek merged 20 commits intocoder:mainfrommtojek:6368-legacy-variable-name
Mar 14, 2023

Conversation

mtojek
Copy link
Member

@mtojekmtojek commentedMar 10, 2023
edited
Loading

Resolves:#6368
Resolves:#6074

This PR extends the "build workspace" logic to pull values from legacy variables to make the parameter migration smooth.

I added a relatively long unit test to cover the whole migration path (pulling value from legacy parameter).

@mtojekmtojek self-assigned thisMar 10, 2023
@aaronlehmann
Copy link
Contributor

Is there a supported migration path forlist(string) variables, or is it best to create a new string parameter (for example, CSV-formatted) and support both the legacy variable and the new parameter during the migration period?

@mtojek
Copy link
MemberAuthor

Is there a supported migration path for list(string) variables, or is it best to create a new string parameter (for example, CSV-formatted)

So far we don't plan to add more types tocoder_parameter, so you may want to "downgrade" to an ordinary string. BTW if you find it useful, please open an issue, describe your use case, and we can review it together.

support both the legacy variable and the new parameter during the migration period?

I will improve docs here, and describe the recommended workflow as part of#6074. Is this parameter intended to be modified by users? Maybe you want to rather keep it as managed Terraform variable.

@mtojekmtojek marked this pull request as ready for reviewMarch 13, 2023 13:03
@aaronlehmann
Copy link
Contributor

So far we don't plan to add more types tocoder_parameter, so you may want to "downgrade" to an ordinary string. BTW if you find it useful, please open an issue, describe your use case, and we can review it together.

The existing variable is used to specify a free-form list of security groups for the workspace. I had planned to convert this to a comma-separated string when I transition to rich parameters, but I'm a little unclear on the migration path - it sounds I will need my own logic to pull the value from either thecoder_parameter or the TF variable, as applicable.

I will improve docs here, and describe the recommended workflow as part of#6074. Is this parameter intended to be modified by users? Maybe you want to rather keep it as managed Terraform variable.

Yes, the parameter is for users to modify. I thought that support for legacy variables was planned to be removed in April?

@mtojek
Copy link
MemberAuthor

The existing variable is used to specify a free-form list of security groups for the workspace.

Well, so far we can supportstring + single choice options,bool, andnumber types.

but I'm a little unclear on the migration path - it sounds I will need my own logic to pull the value from either the coder_parameter or the TF variable, as applicable.

I described the migration stepshere. As you can see, you can configurelegacy_variable_name andlegacy_variable to point to the legacy parameter, and make the migration smoother.

Yes, the parameter is for users to modify. I thought that support for legacy variables was planned to be removed in April?

That is correct, but it doesn't mean that we will remove variables from Terraform templates. Those will be still available to use viamanaged Terraform variables (see:#5980).

@aaronlehmann
Copy link
Contributor

but I'm a little unclear on the migration path - it sounds I will need my own logic to pull the value from either the coder_parameter or the TF variable, as applicable.

I described the migration stepshere. As you can see, you can configurelegacy_variable_name andlegacy_variable to point to the legacy parameter, and make the migration smoother.

Right, but I'm assuming that won't work for alist(string) legacy variable. Is that incorrect?

Yes, the parameter is for users to modify. I thought that support for legacy variables was planned to be removed in April?

That is correct, but it doesn't mean that we will remove variables from Terraform templates. Those will be still available to use viamanaged Terraform variables (see:#5980).

The link says "Workspace users are not able to modify template variables", so it sounds ilke I will need to convert this to acoder_parameter one way or another.

Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏻, mainly had some feedback on the docs language.

Comment on lines 224 to 242
1. Prepare and update a new template version:

- Add `coder_parameter` resource matching the legacy parameter to migrate.
- Use `legacy_variable_name` and `legacy_variable` to link both.
- Mark the new parameter as `mutable`, so that Coder will not block updating existing workspaces.

2. Update all workspaces to the uploaded template version. Coder will populate `coder_parameter`s with values from legacy parameters.
3. Prepare another template version:

- Remove migrated variable.
- Remove properties `legacy_variable` and `legacy_variable_name` from `coder_parameter`s.

4. Update all workspaces to the uploaded template version.
5. Prepare another template version:

- Enable the `feature_use_managed_variables` provider flag to use managed Terraform variables for template customization. Once the flag is enabled, legacy parameters won't be used.

6. Update all workspaces to the uploaded template version.
7. Delete legacy parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. Prepare and update a new template version:
- Add`coder_parameter` resource matching the legacy parameter to migrate.
- Use`legacy_variable_name` and`legacy_variable` to link both.
- Mark the new parameter as`mutable`, so that Coder will not block updating existing workspaces.
2. Update all workspaces to the uploaded template version. Coder will populate`coder_parameter`s with values from legacy parameters.
3. Prepare another template version:
- Remove migrated variable.
- Remove properties`legacy_variable` and`legacy_variable_name` from`coder_parameter`s.
4. Update all workspaces to the uploaded template version.
5. Prepare another template version:
- Enable the`feature_use_managed_variables` provider flag to use managed Terraform variables for template customization. Once the flag is enabled, legacy parameters won't be used.
6. Update all workspaces to the uploaded template version.
7. Delete legacy parameters.
1. Prepare and update a new template version:
- Add `coder_parameter` resource matching the legacy parameter to migrate.
- Use `legacy_variable_name` and `legacy_variable` to link both.
- Mark the new parameter as `mutable`, so that Coder will not block updating existing workspaces.
2. Update all workspaces to the uploaded template version. Coder will populate`coder_parameter`s with values from legacy parameters.
3. Prepare another template version:
- Remove migrated variable.
- Remove properties `legacy_variable` and `legacy_variable_name` from `coder_parameter`s.
4. Update all workspaces to the uploaded template version.
5. Prepare another template version:
- Enable the `feature_use_managed_variables` provider flag to use managed Terraform variables for template customization. Once the flag is enabled, legacy parameters won't be used.
6. Update all workspaces to the uploaded template version.
7. Delete legacy parameters.

I think this might be a bit easier to follow (indented bullet points).

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Fixed!

mtojekand others added9 commitsMarch 14, 2023 10:56
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
@mtojekmtojekenabled auto-merge (squash)March 14, 2023 10:07
@mtojekmtojek merged commit7587850 intocoder:mainMar 14, 2023
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsMar 14, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@mafredrimafredrimafredri approved these changes

@kylecarbskylecarbsAwaiting requested review from kylecarbs

Assignees

@mtojekmtojek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rich parameters: import values from legacy parameters docs: rich parameters
3 participants
@mtojek@aaronlehmann@mafredri

[8]ページ先頭

©2009-2025 Movatter.jp