-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Make sure dashboard is deployed after server and public api server #18995
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
Conversation
61f3551
to
68e6562
Compare
943f4c1
to
3dee5ef
Compare
// We need a new service account because | ||
// - Update old one will make preview env / dedicated deploy failed with err | ||
// The RoleBinding "dashboard" is invalid: roleRef: Invalid value: rbac.RoleRef{APIGroup:"rbac.authorization.k8s.io", Kind:"Role", Name:"dashboard"}: cannot change roleRef | ||
// - Add new one will not work for dedicated since it will not clean old resources |
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'm not sure I understand this line: AFAIK with this approach, yes, we will leak resources (which is a more general problem), but the rollout should work, no? 🤔
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.
@geropl We just need a new name, we use Component
as it's name before with xxx-restricted-root-user
as its role (which its actual role was deleted in previous PR but not delete this role bind)
- Change RoleRef is not allowed for k8s
- When we use APIs, it will check the bind roles, then find that
xxx-restricted-root-user
not exists, so we need to delete old one
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.
- we will leak resources : no
- rollout should work: yes
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.
Cool!
/gh run recreate-vm=true |
aae2bf5
to
9d8e15a
Compare
Let's hold until Monday |
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 only looked at the code and did not try.
Description
To make sure dashboard is deployed after server and public api server
server
orpublic-api-server
deployed a new version, dashboard will restart. But it's not harmful for dashboard.server
orpublic-api-server
's image anymore. Everything that deployed should be built from installerSummary generated by Copilot
### 🤖 Generated by Copilot at d93f3faThis pull request adds a new
service-waiter
package and command that waits for theserver
andpublic-api-server
components to be ready before starting thedashboard
component. It also adds a new service account and role for thedashboard
component, and updates the constants and deployments of thedashboard
andpublic-api-server
components accordingly.Related Issue(s)
Fixes EXP-822
How to test
server-waiter
andpublic-api-server-waiter
, they should be succeed anddashboard
should be accessable--image
of dashboard initContainer should be the same withserver
andpublic-api-server
's image.eu.gcr.io/gitpod-core-dev/build/server:main-gha.19119
, restart dashboard, it should failed. (remember to undo server changes)✅ Test result share
Documentation
Preview status
Gitpod was successfully deployed to your preview environment.
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-test
Publish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/preview
If enabled this will create the environment on GCE infra
Valid options are
all
,workspace
,webapp
,ide
,jetbrains
,vscode
,ssh
. If enabled,with-preview
andwith-large-vm
will be enabled./hold