Skip to content

Allow workspace names to be editable #3000

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Tracked by #3042 ...
sreya opened this issue Jul 15, 2022 · 34 comments · Fixed by #3409
Closed
Tracked by #3042 ...

Allow workspace names to be editable #3000

sreya opened this issue Jul 15, 2022 · 34 comments · Fixed by #3409
Assignees

Comments

@sreya
Copy link
Collaborator

sreya commented Jul 15, 2022

Problem

I want to move my primary workspace off the coder template to the coder-ts template that Ammar created. The only problem is I obviously can't use my primary workspace name dev for both workspaces.

Definition of done

It would be nice if I could rename my obsolete workspace and claim dev for my new workspace.

@sharkymark
Copy link
Contributor

I want this. There should be a pencil icon next to the workspace name to change this.

@ammario
Copy link
Member

ammario commented Jul 18, 2022

Related #2877

@ammario
Copy link
Member

ammario commented Jul 18, 2022

As apart of this, we should suggest that persistent resources are named using an ID.

@ketang
Copy link
Contributor

ketang commented Jul 18, 2022

Completely arbitrary/random, deterministic, or some combination?

@ammario
Copy link
Member

ammario commented Jul 18, 2022

Well, just something stable with the lifecycle of the workspace, so random would be good.

If we allow workspace renames I think all existing templates would still work, it's just that your persistent volume would be named inconsistently from the workspace, leading to confusion and conflict risk.

@kylecarbs
Copy link
Member

All resources in the database have a UUIDv4 associated right now. This isn't editable right now just because we haven't done it.

@ammario
Copy link
Member

ammario commented Jul 18, 2022

And I suppose we should require that the workspace is offline for a rename to occur to avoid a situation where infrastructure names are misaligned with Coder names.

@kylecarbs
Copy link
Member

Now that's a fun problem. The same exists with a username.

@sreya
Copy link
Collaborator Author

sreya commented Jul 18, 2022

And I suppose we should require that the workspace is offline for a rename to occur to avoid a situation where infrastructure
names are misaligned with Coder names.

Should that matter? Shouldn't we be referencing resources with a parameter that's static for the lifecycle of the resource?

@kylecarbs
Copy link
Member

Agreed @sreya. We shouldn't require a workspace is offline for that reason.

@ammario
Copy link
Member

ammario commented Jul 18, 2022

Do y'all mean that the infrastructure names are static for the lifetime of the workspace? So if I rename my workspace the infrastructure name doesn't change? Beyond the potential to be confusing, this would create a conflict bug when a new workspace is created with the old name on the same template.

@ketang
Copy link
Contributor

ketang commented Jul 18, 2022

I think semi-deterministic has value for auditing and cleanup. Completely arbitrary/random IDs are more opaque.

So if I rename my workspace the infrastructure name doesn't change?

Hmmm... and what about rebuilds 🤔 ?

@ammario
Copy link
Member

ammario commented Jul 18, 2022

I think semi-deterministic has value for auditing and cleanup. Completely arbitrary/random IDs are more opaque.

What are the inputs to a deterministic ID?

@sreya
Copy link
Collaborator Author

sreya commented Jul 18, 2022

Do y'all mean that the infrastructure names are static for the lifetime of the workspace? So if I rename my workspace the
infrastructure name doesn't change?

I know that for example in v1 we used the human-friendly name of the pod + some suffix that k8s generates to ensure uniqueness, but to search for it we always queried for the label with the ID of the workspace. So if you updated the name of your workspace, it didn't affect our ability to track the underlying compute.

Looking at v1 funny enough we don't actually allow you to update names but I'm pretty sure things are designed that enabling that should work.

@ammario
Copy link
Member

ammario commented Jul 18, 2022

A big difference is that the template author chooses the exact naming scheme in v2. So, allowing the workspace is be online during name change also means complicating the UX around templates. For example, this pattern becomes a bug risk:

name = "coder-${data.coder_workspace.me.owner}-${lower(data.coder_workspace.me.name)}"

@sreya
Copy link
Collaborator Author

sreya commented Jul 18, 2022

Yeah I suppose if we don't have some mechanism for plugging in our own "source of truth" for identifying workspaces and have to rely on user-submitted parameters then we shouldn't allow renaming workspaces and file that as a design decision.

@sreya
Copy link
Collaborator Author

sreya commented Jul 18, 2022

Should we close this issue then?

@ammario
Copy link
Member

ammario commented Jul 18, 2022

Well it could still work if the persistent resources use ID, for example:

resource "docker_volume" "home_volume" {
  name = "coder-${data.coder_workspace.me.id}-home"
}

@kylecarbs kylecarbs mentioned this issue Jul 19, 2022
20 tasks
@ketang
Copy link
Contributor

ketang commented Jul 19, 2022

What are the inputs to a deterministic ID?

We could use other IDs, e.g. user ID, like in your example above. Changeable human readable names are a risk. For cleanup, if it's not too long, user ID, template ID, workspace ID, and workspace generation number would be good. That's a whole lot of gUUID-ness, though. We could do the first or last N digits of those to make it shorter.

@ammario
Copy link
Member

ammario commented Jul 19, 2022

We're talking specifically about the generation of the workspace ID. I don't see any obvious approach to doing that deterministically (barring a timestamp or sequence number which goes against the spirit). You could use $checksum(userID \cdot templateID)$ but that would break with multiple workspaces for the same template. Also, I find that most tools use UUIDs as a default. What do you think makes this use case special?

@ketang
Copy link
Contributor

ketang commented Jul 19, 2022

The specific area I'm concerned about is the possibility of leaking expensive downstream resources and making the cleanup easier. Perhaps there's a better way to track down orphaned resources, but naming seemed like something that would work and be relatively general.

@ammario
Copy link
Member

ammario commented Jul 19, 2022

Ok, so you want a deterministic element of the ID, not necessarily the entire ID be deterministic. I can see the value in that. We could generate the workspace ID from fragments of the template ID and user ID concatenated with - then.

@ammario
Copy link
Member

ammario commented Jul 19, 2022

I think that is a separate concern, as the rename feature doesn't impact the orphanage rates. I made #3048 to track that.

@ketang
Copy link
Contributor

ketang commented Jul 19, 2022

so you want a deterministic element of the ID, not necessarily the entire ID be deterministic

Exactly. You implied this, but I want to make it explicit: "a deterministic and unique element of the ID"

@tjcran
Copy link

tjcran commented Jul 21, 2022

Duplicates #2188

@mafredri mafredri self-assigned this Jul 22, 2022
@mafredri
Copy link
Member

mafredri commented Jul 26, 2022

Would it be unfeasible for a rename to simply be a surface-level change that does not release the underlying name the workspace was created with?

For instance, say you have a workspace named work, you rename it other. Now both work and other are occupied, and you can only reach your workspace via other.

If you try to create a new work workspace, an error would tell you the name is occupied by other.

This would avoid issues like needing the workspace to be offline, or renaming resources at a cloud provider (which might not be possible).

If this is a no-go, then I believe we should try to address #3048 before allowing renames.

@kylecarbs
Copy link
Member

I think that could be confusing to users. What if I rename a renamed workspace? Does the old name free up?

I think we can address #3048 first if you feel it'd lead to a better experience, but I don't think it's entirely mandatory. I'm of the perspective a workspace doesn't even need to be stopped to rename.

@tjcran tjcran mentioned this issue Jul 29, 2022
25 tasks
@mafredri
Copy link
Member

mafredri commented Aug 3, 2022

@kylecarbs I started working on this today and observed the issue I had in mind when proposing immutable underlying names, with changeable "pretty names". (This is mostly due to the limitation of how workspaces are created today).

Case 1 (because templates uses data.coder_workspace.me.name as part of the volume name):

  1. A user renames their workspace
  2. The users workspace stops
  3. Terraform deletes the home volume because data.coder_workspace.me.name changed
  4. The users workspace starts and has no data

This is what happens with the docker provider, at least.

In this scenario, if the users workspace was stopped during rename, the delete would happen on start, but in either case, data is lost.

Case 2 (we try to work-around case 1 by using immutable volume names):

  1. A user renames their workspace (say, from current to old)
  2. The current workspace name is freed up (but the volume name still exists)
    1. The user tries to create a workspace named current but there is a naming collision with the volume
    2. Or in the worst case scenario, inherits the volume used by old

These issues could be worked around if providers allowed us to rename resources, but we can't expect that to be the case for all of them (and that would require user configuration).

We can work around all these issues by using data.coder_workspace.me.id instead of data.coder_workspace.me.name, however, current users are at risk of data loss.

To ensure no data is lost, either 3i) has to happen or we permanently reserve the name for any non-deleted workspaces.

@kylecarbs
Copy link
Member

I think we shouldn't promote names being used, and instead user IDs should be. This property is already exposed to the Terraform too.

This will probably bite some people, and we can work to improve our updates separately by showing a preview of a dry run. I don't think we should try to do anything magical with the variables though, because there will always be edge-cases we can't fix.

@mafredri
Copy link
Member

mafredri commented Aug 3, 2022

I'm all for changing (and promoting) IDs in the templates, I think that's great. What I worry about is that this will bite every one of our current users. A breaking change would be fine but data loss is more severe IMO.

Addendum: Every one using the rename feature at least. But I imagine this will likely happen after template updates as well where the user is keeping their templates in sync with our sample ones.

@kylecarbs
Copy link
Member

We could implement a check if the workspace was created before a specific time, and warn the user.

@ammario
Copy link
Member

ammario commented Aug 3, 2022

We could implement a check if the workspace was created before a specific time, and warn the user.

👍🏽

Right now is the time to break things.

@mafredri
Copy link
Member

mafredri commented Aug 5, 2022

Since the problem I raised here is a bit more involved (not just limited to renames), I've opened a separate issue to track it: #3386.

@mafredri
Copy link
Member

This has been merged with the caveat that renaming will almost definitely result in data loss. For this reason, coder rename is a hidden command and has an annoying confirmation prompt.

For renaming to be useful, step 1 is for #3663 to be merged (and users ensuring their volume names are immutable).

But ultimately, we'd want #3386 fixed so that renaming is safe.

mafredri added a commit that referenced this issue Nov 8, 2022
mafredri added a commit that referenced this issue Nov 9, 2022
* fix: Use immutable names for volumes in example templates

This contributes towards #3000, #3386

Related #3409

* Add lifecycle and labels
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 a pull request may close this issue.

8 participants