bhouse-nexthop opened a new pull request, #14634: URL: https://github.com/apache/cloudstack/pull/14634
### Description A CKS cluster created before 4.21 has no control, worker or etcd node template set. The 4.21 upgrade added those columns (#9102) and left them NULL. The action workers looked the node templates up from those columns only, so scaling up such a cluster passed a null template to the VM deployment, which failed with a NullPointerException. Fixes #14628 - `KubernetesClusterActionWorker.init()`: a node type with no template of its own now uses `clusterTemplate`, the template CKS picks by default. That is the template these clusters deployed their nodes from before 4.21. Clusters created on 4.21 or later always have the control and worker templates set, and the etcd template whenever they have etcd nodes, so nothing changes for them. - `isDefaultTemplateUsed()`: a cluster with no control or worker template now counts as using the default template, so the owner gets launch permission for `clusterTemplate` while nodes are deployed, and loses it afterwards, as before 4.21. Before this change it returned false for these clusters, as none of the three columns equals `template_id`. Why `clusterTemplate` and not the cluster's `template_id`: for a pre-4.21 cluster, `template_id` is the system VM template that was current when the cluster was created. After the upgrade that is no longer the default, and the launch permission CKS grants is for `clusterTemplate`. A cluster owned by a regular user could then not deploy from `template_id`. Filling the columns in during the upgrade instead would be a schema change, so it would have to target main, and it wouldn't help clusters already upgraded to 4.21 or 4.22. ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [x] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Bug Severity - [ ] BLOCKER - [ ] Critical - [x] Major - [ ] Minor - [ ] Trivial ### How Has This Been Tested? New unit tests in `KubernetesClusterActionWorkerNodeTemplateTest`: - `init()` on a cluster with no node type templates sets the control, worker and etcd templates to the default template, and `isDefaultTemplateUsed()` is true. - `init()` on a cluster with node type templates uses them, and `isDefaultTemplateUsed()` is false when none of them is the cluster's template. - `isDefaultTemplateUsed()` is true when one node type uses the cluster's template. Without the fix, the first test fails. `mvn -pl plugins/integrations/kubernetes-service -am test -Dtest='KubernetesClusterActionWorker*Test'` passes. On a test environment, a cluster created on 4.20 and upgraded to 4.22 failed to scale up with the NullPointerException in #14628. Setting the node type template columns to the cluster's `template_id` by hand made the scale-up work. This PR itself has not been run against a live cluster. #### How did you try to break this feature and the system with this change? - Clusters created on 4.21 or later have their node type templates set, so they take the same path as before. - A 4.21+ cluster with no etcd nodes has no etcd template. `etcdTemplate` is now the default template instead of null, but it is only used to deploy etcd nodes. - External-managed clusters have no templates at all. `isDefaultTemplateUsed()` already returned true for them, as the list of null template ids contained their null `template_id`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
