-
Notifications
You must be signed in to change notification settings - Fork 22
feat: reuse agent tokens when a prebuilt agent reinitializes #374
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is definitely worth an integration test :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this PR is ready for review yet, is it?
"token_provided": existingToken != "", | ||
} | ||
if isPrebuild && existingToken != "" { | ||
// check if a token was already generated for this agent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: consistency.
// check if a token was already generated for this agent. | |
// Check if a token was already generated for this agent. |
} else { | ||
tflog.Info(ctx, "using a new agent token", logFields) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's necessary to log this case.
} else { | |
tflog.Info(ctx, "using a new agent token", logFields) | |
} | |
} |
// RunningAgentTokenEnvironmentVariable returns the name of the environment variable | ||
// that contains the token for the running agent. This is used for prebuilds, where | ||
// we want to reuse the same token for the next iteration of a workspace agent before | ||
// and after the workspace was claimed by a user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment could use some work; I think we should elaborate as to why this is being injected.
// and after the workspace was claimed by a user. | ||
// | ||
// agentID is unused for now, but will be used as soon as we support multiple agents. | ||
func RunningAgentTokenEnvironmentVariable(agentID string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will the ID be injected? How will we maintain a persistent identity across terraform apply
runs?
This PR closes #364