-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat(ws): initial Workspace and WorkspaceKind controller loops #22
feat(ws): initial Workspace and WorkspaceKind controller loops #22
Conversation
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
We can do a proper review of this in our session on Monday, but until then feel free to keep working on it. @jiridanek you also might want to take a look at this (if you were making a similar PR). |
I weren't, in the end. Does it run with
? |
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I have made a significant commit 54e7ed2 to your branch, honestly it's hard to list all the changes, but it should give you a good starting point so we can get closer to mering this. The keys things you need to do are:
I won't do any more commits until our next review, so feel free to make new commits on the branch to move the PR forward. |
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I have made one more change in 01ce3ff Which is to implement:
You should be good to make your own commits now, I dont have any more I need to make in the short term. |
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I have added another change in 5f2597f The changes are:
|
262a435
to
53a900c
Compare
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I have made some changes in a49ffba The key changes are:
Also, please leave the
I have left some TODOs, especially in the tests. Remember that the StatefulSet controller doesnt run in envtest, so the Pods will never actually be created, but we can still test most of it (and do the actual Pod testing in the |
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I have made some changes in 27d03c4 They key changes are:
I will make one more commit, then we can merge this so we can do the TODOs in separate follow up PRs to spread the work out between people. |
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@Adembc I made one last change in aa403b1 The change was to move the default of each option in WorkspaceKind under a spawner section, to help clarify that this is NOT a default on the Workspace, but only for the Spawner UI. It also creates space to do future "display configs" for each option in the Spawner UI. |
@Adembc I am going to merge this now, let's create follow up PRs for the TODOs and tasks in your doc. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR makes the following changes to the workspace controller:
NewControllerManagedBy
to ensure our controller own StatefulSets and Services.generateName
prefix set tows-{WORKSPACE_NAME}-
.Implements retry logic onAlreadyExists
error when creating StatefulSets and Services.