Conversation
mdnix
added
bump:minor
|
🚀 Merging this PR will release Merging will trigger workflows 🛠️ Auto tagging enabled with label |
1 similar comment
|
🚀 Merging this PR will release Merging will trigger workflows 🛠️ Auto tagging enabled with label |
mdnix
force-pushed
the
init-implementation
branch
from
db6dc7c to
6886a05
Compare
mdnix
force-pushed
the
init-implementation
branch
from
6886a05 to
581f350
Compare
There was a problem hiding this comment.
There's a lot of duplication between this component and the CSI driver component, and from what I can see, the two components are part of a single versioned cloud-provider-openstack package.
It might be worth considering merging this component and the CSI driver component into component-cloud-provider-openstack (or similar). While having such components isn't strictly Project Syn best practices, for two controllers which share an upstream repository (https://github.com/kubernetes/cloud-provider-openstack) it would be acceptable (and maybe even preferable) to have a single Commodore component to reduce the amount of duplicated code (which will come with duplicated bug fixing / adjusting for upstream changes).
| local renderCloudConf() = | ||
| std.join( | ||
| '\n', | ||
| renderSection('Global', mergedGlobal) + | ||
| renderSection('Networking', params.cloud_conf.networking) + | ||
| renderSection('LoadBalancer', params.cloud_conf.load_balancer) + | ||
| renderLBClasses(params.cloud_conf.load_balancer_classes) + | ||
| renderSection('Metadata', params.cloud_conf.metadata) + | ||
| renderSection('Route', params.cloud_conf.route) | ||
| ); |
There was a problem hiding this comment.
Same as CSI driver, consider using std.manifestIni().
|
|
||
| [horizontal] | ||
| type:: dictionary | ||
| default:: See `class/defaults.yml` |
There was a problem hiding this comment.
| default:: See `class/defaults.yml` | |
| default:: See https://github.com/projectsyn/component-openstack-cloud-controller-manager/blob/master/class/defaults.yml[`class/defaults.yml`] |
|
|
||
| [horizontal] | ||
| type:: dictionary | ||
| default:: See `class/defaults.yml` |
There was a problem hiding this comment.
| default:: See `class/defaults.yml` | |
| default:: See https://github.com/projectsyn/component-openstack-cloud-controller-manager/blob/master/class/defaults.yml[`class/defaults.yml`] |
| == `credentials` | ||
|
|
||
| [horizontal] | ||
| type:: dictionary | ||
| default:: See `class/defaults.yml` | ||
|
|
||
| Discrete Vault references for sensitive `[Global]` fields. | ||
| At render time, non-null and non-empty entries are merged into `cloud_conf.global` before the INI is emitted. |
There was a problem hiding this comment.
Same as CSI driver: this isn't really necessary, since you can just as well use Vault refs in cloud_conf.global.
There was a problem hiding this comment.
Same as CSI driver: we usually don't strip Helm chart labels from charts rendered via Commodore component.
2 participants
Checklist
changelog.
The PR has a meaningful description that sums up the change. It will be
linked in the changelog.
bug,enhancement,documentation,change,breaking,dependencyas they show up in the changelog.