Gke provisioning #11025
Gke provisioning #11025
Conversation
| @@ -339,7 +339,7 @@ export default { | |||
| </div> | |||
| <div | |||
| v-if="showAdd && !isView" | |||
| class="footer" | |||
| class="footer mt-20" | |||
There was a problem hiding this comment.
Added to make this consistent with keyvalue styling
pkg/gke/util/gcp.ts
Outdated
| export function getGKESharedSubnetworks(store: Store<any>, cloudCredentialId: string, projectId: string, location: {zone?: string, region?: string}): Promise<getGKESharedSubnetworksResponse> { | ||
| // return getGKEOptions('gkeSharedSubnets', store, cloudCredentialId, projectId, location); | ||
| // TODO nb remove this test code | ||
| return Promise.resolve({ |
There was a problem hiding this comment.
This should be removed before the PR is merged. Our dev GCP setup doesn't have permission to create shared subnets - this data was borrowed from rancher/ui#4790
There was a problem hiding this comment.
I've given the functional side a first pass, findings below
- Cloud Credentials
- When creating a
GoogleCloud Credential directly via theCloud Credentialpage no project id is requested, however whilst on the create cloud cred steps when creating a cluster (when no cred found) it is- Is the project id something only storable / discernible in the cluster context?
- When selecting a
GoogleCloud Credential via Cluster Create page- clicking the
Createbutton without going throughAuthenticategives the error notificationcluster kubernetes version cannot be empty string. Can we disable the button if the user hasn't authed? - clicking the
Createbutton without going throughAuthenticate, and then supplying the project id --> authenticate results in the rest of the form showing however the node pools section is empty
- clicking the
- the red notification at top of the create cluster create cloud cred page saying
cloud credential or project id missingpops up in odd ways- on a system with no cloud creds, when going to the cluster create page it's shown before the user hasn't had a chance to enter a project id, and will then blip in/out when the user enters/clears field
- After selecting a Cloud Cred and clicking on
Authenticatethe button width changes to show the spinner which wiggles the two input field widths
- When creating a
- Create
- I created a cluster by adding just a name, no other changes, and it fails to come up
- rancher status
Waiting on gke crd to be initialized, i don't see any clusters in https://console.cloud.google.com/kubernetes/list/overview?authuser=1&project= (is that the right place?)
- rancher status
- I created a cluster by adding just a name, no other changes, and it fails to come up
- Create (form)
logging and monitoring services are hardcoded lists with two options, one of which is none: I'm using a checkbox instead of a dropdown.- Not sure about this suggestion, but should we inform the user of the service names in a tooltip?
- Old UI Cluster Options --> Pod secondary Range Name is missing (but Pod Secondary CIDR Block is shown). Is this related to the
getGKESharedSubnetworkscomment?- Same with missing
Services Secondary Range Name
- Same with missing
- New UI Networking --> Advanced Options
- Can the checkboxes be vertical instead of horizontal? They all seem to affect each other in some way, and it would tie together
CIDR Blockkey value component exposed viaMaster Authorized Network.
- Can the checkboxes be vertical instead of horizontal? They all seem to affect each other in some way, and it would tie together
- After filling in the form and clicking
Createthe contents of the Node Pool tabs is briefly shown with no content - Node Pools
- Old UI
Image Typehas an additional optionWindows Long Term Service Channel with Containerd(zonal, us-central1-c) - API Access Scopes -->
Set access for each API. We've lost horizontal space in the UI given the tab bar. For me full screen this means only one column of select controls are shown. Given the content of these is small can we shrink them width wise to allow more column/s? Think maybe 2, 3 if they don't look squished?
- Old UI
- Detail View - Config
- This shows a red banner
Network Errorand the cloud creds + authenticate button. PressingAuthenticateshows another network error banner. For this use case is there an easy way to avoid the user having to pressAuthenticate?
- This shows a red banner
- Edit
- Same as detail view config (red banner network error)
Bonus Point (not related to PR) - Create EKS --> Cluster Options --> shows a Kube Version of [1.29. Not sure where this comes from in hosted world, but it also happens in master
Edit: I've added a sections for Detail View - Config and Edit
mantis-toboggan-md
force-pushed
the
gke-provisioning-shuffled
branch
from
3b8de48
to
5e7b798
Compare
That's an oversight on my part - the project id isn't required for cloud credential creation, just cluster creation. I've hidden the input while the user is creating cloud credentials.
The cloud credential json has its project id in it, so when the user creates one inline, I've updated the form to automatically populate the project id input with the value from that JSON data (we can't do this in other scenarios because that data is not saved on the cloud credential itself). I opted not to automatically authenticate as well in the event that the user wants to use a different project id (honestly don't know how valid that is, our dev gcp access only has one project, but the old ui allows it)
Above bugs should all be resolved now
Honestly, I'm not sure what a good alternative would be here. Putting the button on a new line when the input widths are already huge looks weird, too.
discussed above offline
I wasn't sure either so I did a little investigating. Googling those service names doesn't turn up much helpful information (nor many direct hits at all) so I'm not sure there would be any value added in showing them to the user. I did however notice that there are a handful of ways that logging and monitoring can be configured in GKE, so I added an info banner clarifying which aspects are enabled with these Rancher options.
My form logic was a bit off here. The Pod/Service Secondary Range Name inputs should be shown whenever a subnet is selected; I was showing them only when the subnet also had secondary ranges defined.
Done - also changed the other networking checkboxes to be vertical for consistency:
Should be fixed now
I see this option in the new UI as well - do you perhaps mean
I've opted for two columns - I found a 2x9 matrix easier to scan through than 3x6
I'll admit, I forgot about the 'view config' mode in general. I've made a handful of small improvements around this: in addition to hiding the authenticate button, I've prevented any of the gcp api calls from running in view mode (no need to show loading spinners for disabled inputs). I also corrected up the display mode of a few inputs when they're disabled, and a few other minor tweaks related to loading gcp data.
I think I fixed this as part of my view config fixes.
There have already been two issues filed about this :( I took a peek and know what's going on, but will add to an EKS-scoped PR. |
There was a problem hiding this comment.
Given the functionality another pass. Almost all issues resolved (i'll start removing them in the next pass)
-
Cloud Credentials
When creating aGoogleCloud Credential directly via theCloud Credentialpage no project id is requested, however whilst on the create cloud cred steps when creating a cluster (when no cred found) it is-
The cloud credential json has its project id in it, so when the user creates one inline, I've updated the form to automatically populate the project id input with the value from that JSON data
- Nice!
-
When selecting aGoogleCloud Credential via Cluster Create pageclicking theCreatebutton without going throughAuthenticategives the error notificationcluster kubernetes version cannot be empty string. Can we disable the button if the user hasn't authed?clicking theCreatebutton without going throughAuthenticate, and then supplying the project id --> authenticate results in the rest of the form showing however the node pools section is empty
the red notification at top of the create cluster create cloud cred page sayingcloud credential or project id missingpops up in odd wayson a system with no cloud creds, when going to the cluster create page it's shown before the user hasn't had a chance to enter a project id, and will then blip in/out when the user enters/clears field
- After selecting a Cloud Cred and clicking on
Authenticatethe button width changes to show the spinner which wiggles the two input field widths-
Honestly, I'm not sure what a good alternative would be here. Putting the button on a new line when the input widths are already huge looks weird, too.
- We could add something like this below, which gives the area enough room so only the button changes and not forms
min-width: 150px; display: flex; align-items: center; justify-content: center;
-
- After adding junk creds the button shows an error state, but with incorrect text colour. Is this something within this PR or a general regression?
-
Create
I created a cluster by adding just a name, no other changes, and it fails to come up
-
Create (form)
logging and monitoring services are hardcoded lists with two options, one of which is none: I'm using a checkbox instead of a dropdown.Old UI Cluster Options --> Pod secondary Range Name is missing (but Pod Secondary CIDR Block is shown). Is this related to thegetGKESharedSubnetworkscomment?- New UI Networking --> Advanced Options
Can the checkboxes be vertical instead of horizontal? They all seem to affect each other in some way, and it would tie togetherCIDR Blockkey value component exposed viaMaster Authorized Network.- Can the
Master Authorized NetworkCIDR Blockcontrol go underneath the checkbox? just to keep them tied together
After filling in the form and clickingCreatethe contents of the Node Pool tabs is briefly shown with no contentNode PoolsOld UIImage Typehas an additional optionWindows Long Term Service Channel with Containerd(zonal, us-central1-c)API Access Scopes -->Set access for each API. We've lost horizontal space in the UI given the tab bar. For me full screen this means only one column of select controls are shown. Given the content of these is small can we shrink them width wise to allow more column/s? Think maybe 2, 3 if they don't look squished?
-
Detail View - Config
This shows a red bannerNetwork Errorand the cloud creds + authenticate button. PressingAuthenticateshows another network error banner. For this use case is there an easy way to avoid the user having to pressAuthenticate?- Console in error when going to page
-
entry-helpers.js:53 TypeError: Cannot read properties of undefined (reading 'createSubnetwork') at Proxy.render (CruGKE.vue:465:1)
-
-
Detail View - Node list
- Scale down failed with
Cannot read properties of undefined (reading 'includes'). This points to somewhere that isn't hit when adding a breakpoint to and trying again (handleSpoofedRequest
- Scale down failed with
-
Edit
Same as detail view config (red banner network error)- On going to
Edit Configthe first page has some floating buttons - Think this has a similar issue to edit EKS. Existing node pools cannot have their name set (disabled - expected) however new node pools also can't (no node pool name)
Bonus Point (not related to PR) - Create EKS --> Cluster Options --> shows a Kube Version of [1.29. Not sure where this comes from in hosted world, but it also happens in master
Edit: Added scale down issue
...why didn't I think of that? Done.
This was caused by using the
Sure. I played around with a few different arrangements and landed on this (screen recordings are ~1080p window, for context): Screen.Recording.2024-06-14.at.1.06.13.PM.movI don't have strong feelings on these conditionally rendered inputs being to the side vs below the checkboxes, but I will note that they don't work well stacked below the checkboxes; one or the other will be fully out out view when enabling private cluster: Screen.Recording.2024-06-14.at.12.43.08.PM.movI also updated the text on the add button to align better with what that keyvalue component corresponds to in the GCP ui.
Fixed by adding the
I've added a fix to all 3 hosted providers. SelectCredential rendered before a
I'm not seeing this issue in GKE - new node pools in a provisioned cluster can have their name altered; existing pools can have their name altered only if the cluster has not finished provisioning. I have however moved 'group details' to the top of the node tab, to be consistent with the other two providers. |
I don't think that button should be shown at all for kontainer driver clusters: these are node groups but they're being shown as individual nodes that can be scaled down individually. I don't think this PR introduced the bug; I see the issue on master as well. I dont see the bug in 2.8-head however so it is some kinda regression. I'll keep investigating next week, but would prefer to tackle this in a separate PR since it's a shared component and the problem exists across kontainer driver clusters. #11246 |
There was a problem hiding this comment.
Just some new (in review, not regressions given pr changes) edit items remaining
- Cloud Credentials
- Nowt
- Create (form)
- Nowt
- Detail View - Config
- Nowt
- Detail View - Node list
- Nowt
- Edit
Think this has a similar issue to edit EKS. Existing node pools cannot have their name set (disabled - expected) however new node pools also can't (no node pool name)- No longer seeing, or just dreamt it
- Copying in the project id every time get's a bit tiresome. Is there anything we can do to improve this?
- does it exist anywhere in the cluster (i think you confirmed already it's not in the cloud cred)?
- would we be able to provide a list for the user to select?
- Going to edit a single node pool cluster (with default/highest kube version), add a new node pool, Node Details kube version is replaced by check box with missing data
Upgrade the node pool version from to 1.29.5-gke.1121000- Not sure on the mechanism, but for the no-op world we should hide the checkbox (but show an empty section?)
- existing values of gkeConfig.locations are ignored
- cluster in region europe-west1. in edit page only europe-west1-b is shown selected (even when gkeConfig.locations contains 3 entries "europe-west1-b", "europe-west1-c", "europe-west1-d"). 'detail view - config' no extra zones are shown.
There was a problem hiding this comment.
Just some new (in review, not regressions given pr changes) edit items remaining
- Edit
- Copying in the project id every time get's a bit tiresome. Is there anything we can do to improve this?
- Not sure what was happening before, but i see this there on edit now. for create it's fine leaving out for the mo
Going to edit a single node pool cluster (with default/highest kube version), add a new node pool, Node Details kube version is replaced by check box with missing dataexisting values of gkeConfig.locations are ignored
- Copying in the project id every time get's a bit tiresome. Is there anything we can do to improve this?
pkg/gke/components/Config.vue
Outdated
| }); | ||
| } | ||
| if (this.region) { | ||
| const out = this.zonesByRegion[this.region] || []; |
There was a problem hiding this comment.
only applicable if making another change, this can just be straight return d
Thanks for reminding me; I've filed a backend issue rancher/rancher#45880 hopefully this is something we can add for 2.10... It should be possible to list projects through the gcp API, but we're using a subset of available gcp api options through the norman UI (similar to AKS) so we need support added there. Auto-filling when they createa a credential or edit a cluster is the most we can do on our side without that additional api call. |
mantis-toboggan-md
force-pushed
the
gke-provisioning-shuffled
branch
from
cdf25fe
to
1d21602
Compare
…g and location config
mock gcp data networking tests config tests fix upgrade np version checkbox fix account access on edit typescript errors authscopes util tests
…ock saving cluster until project id is authenticated
…l component inputs
mantis-toboggan-md
force-pushed
the
gke-provisioning-shuffled
branch
from
1d21602
to
387d1dd
Compare
* gke provisioning without validation or tests * move version from config tab to location tab, rename config networking and location config * add form validation * sort en-us mock gcp data networking tests config tests fix upgrade np version checkbox fix account access on edit typescript errors authscopes util tests * add private nodes warning banner and fix accountaccess on create * lint * update yarn lock to include ipaddr.js * fix e2e test * testing credential bug * fix selectcredential not allowing new credential to be created * automatically use project id when a new gcp credential is created; block saving cluster until project id is authenticated * logging banner * fix networking when subnet ha sno secondary ip ranges * add svc account fetch and some node pool tests * fix auth scopes styling and view config mode * review feedback and ts errors * no auto-authenticate * fix lint * update networking input placement * add loading component to crugke, crueks, cruaks; shuffle gke node pool component inputs * fix lint and loading import * fix extra zone checkboxes on edit and add unit test * fix gke node pool version checkbox when adding pools * fix lint * fix extrazoneoptions display in view config mode * clean up extraZoneOptions
Summary
Fixes #8967 #10621
Occurred changes and/or fixed issues
Functionality should be on par with existing GKE provisioning in the Ember UI. Some Ember validation happens at same time: it's all form validation in this port. There are a handful of areas where I have deviated from Ember:
excusesjustification for this is detailed in inline commentsAreas or cases that should be tested
Areas which could experience regressions
This PR is pretty self-contained and should not impact areas outside of GKE provisioning. There is a minor stylistic update to ArrayList so it matches KeyValue (both are used next to eachother in node pools and the difference in spacing was apparent, and ugly)
Screenshot/Video
Checklist