Conversation
It was discussed that we probably don't need this filter in the future. According to internal sources the filter was introduced to support the transition of old images to new images needing this trait. Since, now, all hosts provide this trait, it's unnecessary to keep this filter.
This adds basic committed resource (CR) functionality: - update reservations with AZ - reservation label - flavor group knowledge - knowledge with contentChanged - commitments syncer works with flavor group CRs - commitments API endpoint for batch update of CRs - commitments API version endpoint - skeletons for capacity endpoint
Was defined twice in `values.yaml`
With the new use case of the decision crd (for now history crd) we can't use the visualizer anymore. Goal is to make the explanation already good enough :D
…tor digest to 05f22f6 (#573) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/cobaltcore-dev/openstack-hypervisor-operator](https://redirect.github.com/cobaltcore-dev/openstack-hypervisor-operator) | require | digest | `733c59b` → `05f22f6` | --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker/setup-qemu-action](https://redirect.github.com/docker/setup-qemu-action) | action | major | `v3` → `v4` | --- ### Release Notes <details> <summary>docker/setup-qemu-action (docker/setup-qemu-action)</summary> ### [`v4`](https://redirect.github.com/docker/setup-qemu-action/compare/v3...v4) [Compare Source](https://redirect.github.com/docker/setup-qemu-action/compare/v3...v4) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker/setup-buildx-action](https://redirect.github.com/docker/setup-buildx-action) | action | major | `v3` → `v4` | --- ### Release Notes <details> <summary>docker/setup-buildx-action (docker/setup-buildx-action)</summary> ### [`v4`](https://redirect.github.com/docker/setup-buildx-action/compare/v3...v4) [Compare Source](https://redirect.github.com/docker/setup-buildx-action/compare/v3...v4) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker/login-action](https://redirect.github.com/docker/login-action) | action | major | `v3` → `v4` | --- ### Release Notes <details> <summary>docker/login-action (docker/login-action)</summary> ### [`v4`](https://redirect.github.com/docker/login-action/compare/v3...v4) [Compare Source](https://redirect.github.com/docker/login-action/compare/v3...v4) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker/build-push-action](https://redirect.github.com/docker/build-push-action) | action | major | `v6` → `v7` | --- ### Release Notes <details> <summary>docker/build-push-action (docker/build-push-action)</summary> ### [`v7`](https://redirect.github.com/docker/build-push-action/compare/v6...v7) [Compare Source](https://redirect.github.com/docker/build-push-action/compare/v6...v7) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Adoption](https://docs.renovatebot.com/merge-confidence/) | [Passing](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | Type | Update | |---|---|---|---|---|---|---|---| | [github.com/gophercloud/gophercloud/v2](https://redirect.github.com/gophercloud/gophercloud) | `v2.10.0` → `v2.11.1` |  |  |  |  | require | minor | | [k8s.io/api](https://redirect.github.com/kubernetes/api) | `v0.35.1` → `v0.35.2` |  |  |  |  | require | patch | | [k8s.io/apimachinery](https://redirect.github.com/kubernetes/apimachinery) | `v0.35.1` → `v0.35.2` |  |  |  |  | require | patch | | [k8s.io/client-go](https://redirect.github.com/kubernetes/client-go) | `v0.35.1` → `v0.35.2` |  |  |  |  | require | patch | | [kube-prometheus-stack](https://redirect.github.com/prometheus-operator/kube-prometheus) ([source](https://redirect.github.com/prometheus-community/helm-charts)) | `82.4.1` → `82.10.3` |  |  |  |  | | minor | | [sigs.k8s.io/controller-runtime](https://redirect.github.com/kubernetes-sigs/controller-runtime) | `v0.23.1` → `v0.23.3` |  |  |  |  | require | patch | --- ### Release Notes <details> <summary>gophercloud/gophercloud (github.com/gophercloud/gophercloud/v2)</summary> ### [`v2.11.1`](https://redirect.github.com/gophercloud/gophercloud/releases/tag/v2.11.1) [Compare Source](https://redirect.github.com/gophercloud/gophercloud/compare/v2.11.0...v2.11.1) #### What's Changed - \[v2] Do not specify go patch version by [@​mandre](https://redirect.github.com/mandre) in [#​3648](https://redirect.github.com/gophercloud/gophercloud/pull/3648) **Full Changelog**: <gophercloud/gophercloud@v2.11.0...v2.11.1> ### [`v2.11.0`](https://redirect.github.com/gophercloud/gophercloud/releases/tag/v2.11.0) [Compare Source](https://redirect.github.com/gophercloud/gophercloud/compare/v2.10.0...v2.11.0) #### What's Changed - \[v2] Add PCIAddress field to baremetal InterfaceType by [@​MahnoorAsghar](https://redirect.github.com/MahnoorAsghar) in [#​3602](https://redirect.github.com/gophercloud/gophercloud/pull/3602) - \[v2] Networking V2: Added support for ML2 extension port\_trusted\_vif by [@​dlaw4608](https://redirect.github.com/dlaw4608) in [#​3610](https://redirect.github.com/gophercloud/gophercloud/pull/3610) - \[v2] networking/v2/layer3/routers: Add external gateways management by [@​aldokimi](https://redirect.github.com/aldokimi) in [#​3611](https://redirect.github.com/gophercloud/gophercloud/pull/3611) - \[v2] Use jimmy amphora in octavia job by [@​eshulman2](https://redirect.github.com/eshulman2) in [#​3625](https://redirect.github.com/gophercloud/gophercloud/pull/3625) - \[v2] CI: Fix fwaas jobs by [@​mandre](https://redirect.github.com/mandre) in [#​3632](https://redirect.github.com/gophercloud/gophercloud/pull/3632) - \[v2] Bump go by [@​mandre](https://redirect.github.com/mandre) in [#​3630](https://redirect.github.com/gophercloud/gophercloud/pull/3630) - \[v2] Add a new Ironic field representing node health to Gophercloud by [@​jacob-anders](https://redirect.github.com/jacob-anders) in [#​3629](https://redirect.github.com/gophercloud/gophercloud/pull/3629) - \[v2] Add TSIG key support for OpenStack DNS v2 API by [@​omersch381](https://redirect.github.com/omersch381) in [#​3633](https://redirect.github.com/gophercloud/gophercloud/pull/3633) - \[v2] fix: networkipavailabilities: handle scientific notation in IP counts by [@​ednxzu](https://redirect.github.com/ednxzu) in [#​3640](https://redirect.github.com/gophercloud/gophercloud/pull/3640) - Prepare v2.11.0 by [@​mandre](https://redirect.github.com/mandre) in [#​3641](https://redirect.github.com/gophercloud/gophercloud/pull/3641) **Full Changelog**: <gophercloud/gophercloud@v2.10.0...v2.11.0> </details> <details> <summary>kubernetes/api (k8s.io/api)</summary> ### [`v0.35.2`](https://redirect.github.com/kubernetes/api/compare/v0.35.1...v0.35.2) [Compare Source](https://redirect.github.com/kubernetes/api/compare/v0.35.1...v0.35.2) </details> <details> <summary>kubernetes/apimachinery (k8s.io/apimachinery)</summary> ### [`v0.35.2`](https://redirect.github.com/kubernetes/apimachinery/compare/v0.35.1...v0.35.2) [Compare Source](https://redirect.github.com/kubernetes/apimachinery/compare/v0.35.1...v0.35.2) </details> <details> <summary>kubernetes/client-go (k8s.io/client-go)</summary> ### [`v0.35.2`](https://redirect.github.com/kubernetes/client-go/compare/v0.35.1...v0.35.2) [Compare Source](https://redirect.github.com/kubernetes/client-go/compare/v0.35.1...v0.35.2) </details> <details> <summary>prometheus-community/helm-charts (kube-prometheus-stack)</summary> ### [`v82.10.3`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.10.3) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update kube-prometheus-stack dependency non-major updates by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6726](https://redirect.github.com/prometheus-community/helm-charts/pull/6726) **Full Changelog**: <prometheus-community/helm-charts@kube-prometheus-stack-82.10.2...kube-prometheus-stack-82.10.3> ### [`v82.10.2`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.10.2) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update kube-prometheus-stack dependency non-major updates by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6725](https://redirect.github.com/prometheus-community/helm-charts/pull/6725) **Full Changelog**: <prometheus-community/helm-charts@prometheus-pingdom-exporter-3.4.2...kube-prometheus-stack-82.10.2> ### [`v82.10.1`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.10.1) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.10.0...kube-prometheus-stack-82.10.1) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] add configurable reloaderWebNodePort by [@​firasmosbehi](https://redirect.github.com/firasmosbehi) in [#​6717](https://redirect.github.com/prometheus-community/helm-charts/pull/6717) **Full Changelog**: <prometheus-community/helm-charts@prometheus-sql-exporter-0.5.0...kube-prometheus-stack-82.10.1> ### [`v82.10.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.10.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.9.0...kube-prometheus-stack-82.10.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update kube-prometheus-stack dependency non-major updates by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6722](https://redirect.github.com/prometheus-community/helm-charts/pull/6722) **Full Changelog**: <prometheus-community/helm-charts@prometheus-nginx-exporter-1.20.1...kube-prometheus-stack-82.10.0> ### [`v82.9.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.9.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.8.0...kube-prometheus-stack-82.9.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update kube-prometheus-stack dependency non-major updates by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6706](https://redirect.github.com/prometheus-community/helm-charts/pull/6706) **Full Changelog**: <prometheus-community/helm-charts@prometheus-nginx-exporter-1.20.0...kube-prometheus-stack-82.9.0> ### [`v82.8.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.8.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.7.0...kube-prometheus-stack-82.8.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Add VPA support for Prometheus by [@​QuentinBisson](https://redirect.github.com/QuentinBisson) in [#​6700](https://redirect.github.com/prometheus-community/helm-charts/pull/6700) **Full Changelog**: <prometheus-community/helm-charts@prometheus-nginx-exporter-1.19.5...kube-prometheus-stack-82.8.0> ### [`v82.7.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.7.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.6.1...kube-prometheus-stack-82.7.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Add VPA support for Alertmanager by [@​QuentinBisson](https://redirect.github.com/QuentinBisson) in [#​6699](https://redirect.github.com/prometheus-community/helm-charts/pull/6699) **Full Changelog**: <prometheus-community/helm-charts@kube-prometheus-stack-82.6.1...kube-prometheus-stack-82.7.0> ### [`v82.6.1`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.6.1) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.6.0...kube-prometheus-stack-82.6.1) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update Helm release grafana to v11.2.3 by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6701](https://redirect.github.com/prometheus-community/helm-charts/pull/6701) **Full Changelog**: <prometheus-community/helm-charts@kube-prometheus-stack-82.6.0...kube-prometheus-stack-82.6.1> ### [`v82.6.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.6.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.5.0...kube-prometheus-stack-82.6.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update <https://github.com/etcd-io/etcd> digest to [`b9b15e1`](https://redirect.github.com/prometheus-community/helm-charts/commit/b9b15e1) by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6697](https://redirect.github.com/prometheus-community/helm-charts/pull/6697) **Full Changelog**: <prometheus-community/helm-charts@kube-prometheus-stack-82.5.0...kube-prometheus-stack-82.6.0> ### [`v82.5.0`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.5.0) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.4.3...kube-prometheus-stack-82.5.0) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update kube-prometheus-stack dependency non-major updates by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6696](https://redirect.github.com/prometheus-community/helm-charts/pull/6696) **Full Changelog**: <prometheus-community/helm-charts@prometheus-nginx-exporter-1.19.4...kube-prometheus-stack-82.5.0> ### [`v82.4.3`](https://redirect.github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-82.4.3) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.4.2...kube-prometheus-stack-82.4.3) kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator. #### What's Changed - \[kube-prometheus-stack] Update Helm release grafana to v11.2.2 by [@​renovate](https://redirect.github.com/renovate)\[bot] in [#​6689](https://redirect.github.com/prometheus-community/helm-charts/pull/6689) **Full Changelog**: <prometheus-community/helm-charts@prometheus-yet-another-cloudwatch-exporter-0.42.0...kube-prometheus-stack-82.4.3> ### [`v82.4.2`](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.4.1...kube-prometheus-stack-82.4.2) [Compare Source](https://redirect.github.com/prometheus-community/helm-charts/compare/kube-prometheus-stack-82.4.1...kube-prometheus-stack-82.4.2) </details> <details> <summary>kubernetes-sigs/controller-runtime (sigs.k8s.io/controller-runtime)</summary> ### [`v0.23.3`](https://redirect.github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.23.3) [Compare Source](https://redirect.github.com/kubernetes-sigs/controller-runtime/compare/v0.23.2...v0.23.3) #### What's Changed - 🐛 Ensure DefaulterRemoveUnknownOrOmitableFields is still working even if objects are equal by [@​k8s-infra-cherrypick-robot](https://redirect.github.com/k8s-infra-cherrypick-robot) in [#​3469](https://redirect.github.com/kubernetes-sigs/controller-runtime/pull/3469) **Full Changelog**: <kubernetes-sigs/controller-runtime@v0.23.2...v0.23.3> ### [`v0.23.2`](https://redirect.github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.23.2) [Compare Source](https://redirect.github.com/kubernetes-sigs/controller-runtime/compare/v0.23.1...v0.23.2) #### What's Changed - 🐛 Fix fake client's SSA status patch resource version check by [@​k8s-infra-cherrypick-robot](https://redirect.github.com/k8s-infra-cherrypick-robot) in [#​3446](https://redirect.github.com/kubernetes-sigs/controller-runtime/pull/3446) - ✨ Reduce memory usage of default webhooks by [@​k8s-infra-cherrypick-robot](https://redirect.github.com/k8s-infra-cherrypick-robot) in [#​3467](https://redirect.github.com/kubernetes-sigs/controller-runtime/pull/3467) **Full Changelog**: <kubernetes-sigs/controller-runtime@v0.23.1...v0.23.2> </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/sapcc/go-bits](https://redirect.github.com/sapcc/go-bits) | require | digest | `c20f89b` → `034b497` | --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker/metadata-action](https://redirect.github.com/docker/metadata-action) | action | major | `v5` → `v6` | --- ### Release Notes <details> <summary>docker/metadata-action (docker/metadata-action)</summary> ### [`v6`](https://redirect.github.com/docker/metadata-action/compare/v5...v6) [Compare Source](https://redirect.github.com/docker/metadata-action/compare/v5...v6) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 8am on Friday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cobaltcore-dev/cortex). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42Ni40IiwidXBkYXRlZEluVmVyIjoiNDMuNjYuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
See [CodeRabbit Config docs](https://docs.coderabbit.ai/reference/configuration#reviews)
This upgrade is necessary, because we changed the allocation and capacity types from a generic `map[string]resource.Quantity` to `map[hv1.ResourceName]resource.Quantity`. In addition, this upgrade also makes use of the `hv1.ResourceName` api inside our Reservation CRD. See: cobaltcore-dev/openstack-hypervisor-operator#257 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Standardized resource naming to a strongly-typed format across scheduling and reservation logic, improving type safety and consistency for capacity, allocation and reservation handling. * **Chores** * Bumped the OpenStack Hypervisor Operator dependency to the newer release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
See https://agents.md/#examples -- this file helps agents understand our code, architecture and intentions.
Support server groups in the workload spawner, so we can spawn vms with anti-affinity or affinity rules.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
helm/dev/cortex-prometheus-operator/Chart.yaml (1)
9-13:⚠️ Potential issue | 🟠 MajorAdd the required local chart source comment in the dependency block.
Lines 9-13 define dependencies, but the mandatory
# from: file://../../library/cortex-postgrescomment is missing. Please add it near this dependency declaration to comply with chart policy.As per coding guidelines, "
**/Chart.yaml: Helm Chart.yaml files must include the# from: file://../../library/cortex-postgrescomment pointing to the local chart path".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/dev/cortex-prometheus-operator/Chart.yaml` around lines 9 - 13, The Chart.yaml dependencies block is missing the required local chart source comment; edit the dependencies section in Chart.yaml near the kube-prometheus-stack entry (the dependency with name: kube-prometheus-stack, repository: oci://ghcr.io/prometheus-community/charts, version: 82.10.3) and add the mandatory comment line "# from: file://../../library/cortex-postgres" directly above or next to that dependency declaration so the file includes the required local chart path marker.internal/scheduling/reservations/controller/controller.go (2)
134-152:⚠️ Potential issue | 🟠 MajorTake the merge base before mutating
res.Status.Line 138 updates
res.Status.Hostbefore Line 142 snapshotsold, soclient.MergeFrom(old)is built from already-mutated state and can be a no-op. The host sync will then silently never persist.Suggested fix
- needsStatusUpdate := false - if res.Spec.TargetHost != "" && res.Status.Host != res.Spec.TargetHost { - res.Status.Host = res.Spec.TargetHost - needsStatusUpdate = true - } - if needsStatusUpdate { - old := res.DeepCopy() + needsStatusUpdate := false + old := res.DeepCopy() + if res.Spec.TargetHost != "" && res.Status.Host != res.Spec.TargetHost { + res.Status.Host = res.Spec.TargetHost + needsStatusUpdate = true + } + if needsStatusUpdate { patch := client.MergeFrom(old)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/controller/controller.go` around lines 134 - 152, The merge base is taken after mutating res.Status which makes client.MergeFrom(old) a no-op; call res.DeepCopy() to capture old before modifying res.Status (use old := res.DeepCopy(); patch := client.MergeFrom(old)), then set res.Status.Host = res.Spec.TargetHost and call r.Status().Patch(ctx, &res, patch) as before, preserving the existing error handling for client.IgnoreNotFound(err).
183-199:⚠️ Potential issue | 🔴 CriticalUse correct units when converting resources for Nova API.
Memory and CPU are converted with incorrect scaling factors, causing Nova to receive wrong values:
- CPU:
ScaledValue(resource.Milli)returns millicores (e.g., 4000), but Nova'sVCPUsfield expects whole vCPUs (e.g., 4)- Memory:
ScaledValue(resource.Mega)uses decimal scaling (divides by 1,000,000), losing ~5% of memory for 8 GiB (8589 MB instead of 8192 MB)Meanwhile,
reservation_manager.gostores resources as bytes and whole CPUs viaresource.NewQuantity(), creating a unit mismatch that will cause Nova to reject valid placements.Suggested fix
var memoryMB uint64 if memory, ok := res.Spec.Resources["memory"]; ok { - memoryValue := memory.ScaledValue(resource.Mega) + memoryValue := memory.Value() if memoryValue < 0 { return ctrl.Result{}, fmt.Errorf("invalid memory value: %d", memoryValue) } - memoryMB = uint64(memoryValue) + memoryMB = uint64((memoryValue + 1024*1024 - 1) / (1024 * 1024)) } var cpu uint64 if cpuQuantity, ok := res.Spec.Resources["cpu"]; ok { - cpuValue := cpuQuantity.ScaledValue(resource.Milli) + cpuValue := cpuQuantity.MilliValue() if cpuValue < 0 { return ctrl.Result{}, fmt.Errorf("invalid cpu value: %d", cpuValue) } - cpu = uint64(cpuValue) + if cpuValue%1000 != 0 { + return ctrl.Result{}, fmt.Errorf("invalid cpu value: %d", cpuValue) + } + cpu = uint64(cpuValue / 1000) }Also applies to: 251-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/controller/controller.go` around lines 183 - 199, The memory and CPU conversions use the wrong scaling: change memory conversion in the block that sets memoryMB (reading from res.Spec.Resources["memory"]) to use memory.ScaledValue(resource.Mebi) so you get MiB (8192 for 8GiB), and change CPU conversion (reading from res.Spec.Resources["cpu"]) to compute whole vCPUs by taking cpuQuantity.ScaledValue(resource.Milli) and dividing by 1000 (e.g., vcpus := uint64(cpuValue / 1000)); apply the same fixes to the other conversion block later (lines ~251-259) so values match how reservation_manager.go stores bytes and whole CPUs and validate the resulting integers before returning.
🧹 Nitpick comments (16)
tools/plutono/provisioning/dashboards/cortex-status.json (1)
2321-2330: Both panels now query the same metric with different aggregations.This panel ("Triggered migrations") and the previous panel ("Deschedulings") now both query
cortex_detector_pipeline_run_duration_seconds_count—one usingrate()and this one usingdelta(). While mathematically they show related data (per-second rate vs absolute change), having two panels for the same underlying metric in the same dashboard section may be redundant.Consider whether:
- The panel title "Triggered migrations" accurately reflects what
cortex_detector_pipeline_run_duration_seconds_countmeasures- Both panels are necessary or could be consolidated
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/plutono/provisioning/dashboards/cortex-status.json` around lines 2321 - 2330, The "Triggered migrations" panel is querying the same underlying metric cortex_detector_pipeline_run_duration_seconds_count (using delta()) as the "Deschedulings" panel (which uses rate()), which may be redundant and the title may be inaccurate; inspect the two panels (the one titled "Triggered migrations" with expr=sum by (error) (delta(cortex_detector_pipeline_run_duration_seconds_count{}[2m])) and the "Deschedulings" panel that uses rate(cortex_detector_pipeline_run_duration_seconds_count)) and either rename "Triggered migrations" to reflect it shows absolute count delta, change its query to a different metric/aggregation that actually represents migrations, or consolidate the two panels into a single panel that presents both rate and absolute change (or remove one) so the dashboard section is not duplicating the same underlying signal.internal/scheduling/lib/detector_pipeline.go (1)
82-83: Narrow lock scope to reduce contention.
defer lock.Unlock()keeps the mutex held across logging. Lock only around the shared writes for better parallelism.Suggested refactor
- lock.Lock() - defer lock.Unlock() if err != nil { slog.Error("descheduler: failed to run step", "error", err) + lock.Lock() metricErrLabel = "true" + lock.Unlock() return } slog.Info("descheduler: finished step") + lock.Lock() decisionsByStep[stepName] = decisions + lock.Unlock()Also applies to: 85-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/lib/detector_pipeline.go` around lines 82 - 83, The mutex currently held by lock.Lock() with defer lock.Unlock() spans too much work (including logging); narrow the critical section by replacing defer unlock with an explicit lock.Unlock() immediately after the shared-state writes so logging happens outside the lock. Locate the lock usage (lock.Lock()/defer lock.Unlock()) in detector_pipeline.go (the same pattern around lines 82–83 and 85–90), perform the shared-data updates while locked, call lock.Unlock(), then do any logging or non-shared work outside the lock to reduce contention.internal/scheduling/reservations/commitments/reservation_manager_test.go (1)
84-123: Consider addingTypefield to test reservation specs.The test reservations set the
LabelReservationTypelabel but don't setSpec.Type. While this may work for testing the manager, it's inconsistent with how real reservations would be created where both should be set.♻️ Suggested improvement
{ ObjectMeta: metav1.ObjectMeta{ Name: "commitment-abc123-0", Labels: map[string]string{ v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource, }, }, Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, Resources: map[hv1.ResourceName]resource.Quantity{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/reservation_manager_test.go` around lines 84 - 123, The test reservations in existingReservations include the LabelReservationType label but omit Spec.Type; add the Type field in each v1alpha1.ReservationSpec (e.g. set Spec.Type = v1alpha1.ReservationTypeCommittedResource or the matching enum used with LabelReservationType/ReservationTypeLabelCommittedResource) so the test objects mirror real reservations; update both reservation entries in the existingReservations slice (referencing v1alpha1.ReservationSpec and Spec.Type) to include the correct type value.internal/knowledge/extractor/controller.go (1)
212-212: Useanyinstead ofinterface{}for JSON comparison variables.Modern Go uses
anyas the type alias forinterface{}. Update line 212 to align with repo conventions.♻️ Suggested change
- var oldData, newData interface{} + var oldData, newData any🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/knowledge/extractor/controller.go` at line 212, Replace the use of the older interface{} type with the modern alias any for the JSON comparison variables: update the declaration of oldData and newData (currently declared as interface{}) to use any so the code conforms to repo conventions and Go idioms.internal/scheduling/reservations/commitments/api_report_capacity_test.go (1)
205-206: Replaceinterface{}withanythroughout the test file.The coding guidelines require using
anyinstead ofinterface{}in modern Go code. This applies to all occurrences in the test, including the test struct definition and feature payload declarations.♻️ Proposed fixes
- body interface{} + body any @@ - emptyFeatures := []map[string]interface{}{} + emptyFeatures := []map[string]any{} @@ - features := []map[string]interface{}{ + features := []map[string]any{ @@ - "flavors": []map[string]interface{}{ + "flavors": []map[string]any{ @@ - "largestFlavor": map[string]interface{}{ + "largestFlavor": map[string]any{Applies to lines: 43, 205, 238, 241, 249.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_report_capacity_test.go` around lines 205 - 206, Replace all occurrences of the old empty interface type with the Go alias any in this test file: update test struct field types and all feature payload declarations (e.g. the emptyFeatures variable and other map[string]interface{} uses passed into v1alpha1.BoxFeatureList and similar calls) to use map[string]any and any instead of interface{}, and ensure function calls like v1alpha1.BoxFeatureList receive the updated map[string]any types.internal/scheduling/reservations/commitments/api_change_commitments_test.go (2)
113-117: Usestrings.Containsfor response-body string checks.These assertions already operate on
bodyStr(string), sostrings.Containsis the guideline-compliant choice.As per coding guidelines: "Use `strings.Contains` to check if a string is contained in another string".💡 Proposed fix
import ( "bytes" "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ - if !bytes.Contains([]byte(bodyStr), []byte("Version mismatch")) { + if !strings.Contains(bodyStr, "Version mismatch") { t.Errorf("expected response to contain 'Version mismatch', got: %s", bodyStr) } - if !bytes.Contains([]byte(bodyStr), []byte("12345")) { + if !strings.Contains(bodyStr, "12345") { t.Errorf("expected response to contain request version '12345', got: %s", bodyStr) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_change_commitments_test.go` around lines 113 - 117, Replace the bytes.Contains calls that check the response text with strings.Contains since bodyStr is already a string: update the two assertions that currently call bytes.Contains([]byte(bodyStr), []byte("...")) to strings.Contains(bodyStr, "...") (referencing the bodyStr variable and the two checks for "Version mismatch" and "12345"); also add "strings" to the test imports and remove "bytes" if it becomes unused.
24-223: These endpoint scenarios are good candidates for one table-driven test.The test coverage is relevant, but setup and request boilerplate repeat across cases and can be consolidated into a compact struct-based table test.
Based on learnings: "Applies to **/*_test.go : Use struct-based test cases when applicable, but limit yourself to the most relevant cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_change_commitments_test.go` around lines 24 - 223, Tests TestHandleChangeCommitments_VersionMismatch, TestHandleChangeCommitments_DryRun and TestProcessCommitmentChanges_KnowledgeNotReady repeat setup and request boilerplate; refactor them into a single table-driven test that iterates cases (e.g., name, initial Knowledge object or nil, request JSON, expected HTTP status, expected content-type, expected response fields) and calls HTTPAPI.HandleChangeCommitments for each row; extract common setup into reusable helpers (scheme creation with v1alpha1.AddToScheme, fake client builder, building API struct) and reuse the same httptest request/recorder flow inside the loop, using unique symbols TestHandleChangeCommitments_Table (or consolidate into TestHandleChangeCommitments) and keep assertions per-case driven by fields in the table to preserve the existing checks for version mismatch, dry-run rejection and "caches not ready" responses.internal/scheduling/reservations/commitments/state_test.go (1)
66-253: Consider consolidating the error-path cases into one table-driven test.The current cases are valid, but several follow the same pattern and can be compacted to reduce maintenance overhead.
Based on learnings: "Applies to **/*_test.go : Use struct-based test cases when applicable, but limit yourself to the most relevant cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/state_test.go` around lines 66 - 253, Consolidate repetitive error-path tests into table-driven cases: replace the separate TestFromCommitment_InvalidResourceName, TestFromReservations_EmptyList, TestFromReservations_MixedCommitmentUUIDs, TestFromReservations_NonCommittedResourceType, and TestGetFlavorGroupNameFromResource_Invalid with one or two table-driven tests that enumerate input name/payload and the function to call (FromCommitment, FromReservations, getFlavorGroupNameFromResource) plus expected error; implement a loop that runs each case, invokes the appropriate symbol (FromCommitment, FromReservations, getFlavorGroupNameFromResource) with the given input, and asserts an error was returned, keeping the existing helper inputs (e.g., commitment structs and reservation slices) as inline case fields for clarity.internal/scheduling/reservations/commitments/syncer.go (1)
196-214: Consider handling NotFound errors during deletion.When deleting reservations for expired commitments, a NotFound error could occur if the reservation was already deleted by another process. This would cause the sync to fail unnecessarily.
Proposed fix to handle NotFound gracefully
if !activeCommitments[commitmentUUID] { // This commitment no longer exists, delete the reservation if err := s.Delete(ctx, &existing); err != nil { + if apierrors.IsNotFound(err) { + log.Info("reservation already deleted", "name", existing.Name) + continue + } log.Error(err, "failed to delete reservation", "name", existing.Name) return err }You'll need to add the import:
apierrors "k8s.io/apimachinery/pkg/api/errors"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/syncer.go` around lines 196 - 214, The deletion loop in the syncer currently returns an error if s.Delete(ctx, &existing) fails, which should ignore NotFound errors; update the block that calls s.Delete (inside the loop iterating existingReservations.Items and using extractCommitmentUUID and activeCommitments) to treat apierrors.IsNotFound(err) as a non-fatal condition (log at info/debug and continue) and only return for other errors, and add the import alias apierrors "k8s.io/apimachinery/pkg/api/errors".internal/scheduling/reservations/commitments/api_change_commitments.go (3)
225-225: Use camelCase for variable name.Variable
time_startshould betimeStartto follow Go naming conventions.Proposed fix
- time_start := time.Now() + timeStart := time.Now() if err := watchReservationsUntilReady(ctx, log, api.client, reservationsToWatch, watchTimeout); err != nil { log.Info("reservations failed to become ready, initiating rollback", "reason", err.Error()) resp.RejectionReason = fmt.Sprintf("Not all reservations can be fulfilled: %v", err) requireRollback = true } - log.Info("finished watching reservation", "totalSchedulingTimeSeconds", time.Since(time_start).Seconds()) + log.Info("finished watching reservation", "totalSchedulingTimeSeconds", time.Since(timeStart).Seconds())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_change_commitments.go` at line 225, The variable name time_start violates Go camelCase conventions; rename the variable time_start to timeStart where it is declared (e.g., in the block containing the time_start := time.Now() statement) and update every use/reference in the same function/scope (search for time_start) to the new identifier timeStart so the code follows Go naming style and compiles cleanly.
163-179: Use camelCase for variable names.Variable names
all_reservationsandexisting_reservationsuse snake_case which is not idiomatic Go. UseallReservationsandexistingReservationsinstead.Proposed fix for naming conventions
// List all committed resource reservations, then filter by name prefix - var all_reservations v1alpha1.ReservationList - if err := api.client.List(ctx, &all_reservations, client.MatchingLabels{ + var allReservations v1alpha1.ReservationList + if err := api.client.List(ctx, &allReservations, client.MatchingLabels{ v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource, }); err != nil { resp.RejectionReason = fmt.Sprintf("failed to list reservations for commitment %s: %v", commitment.UUID, err) requireRollback = true break ProcessLoop } // Filter by name prefix to find reservations for this commitment namePrefix := fmt.Sprintf("commitment-%s-", string(commitment.UUID)) - var existing_reservations v1alpha1.ReservationList - for _, res := range all_reservations.Items { + var existingReservations v1alpha1.ReservationList + for _, res := range allReservations.Items { if len(res.Name) >= len(namePrefix) && res.Name[:len(namePrefix)] == namePrefix { - existing_reservations.Items = append(existing_reservations.Items, res) + existingReservations.Items = append(existingReservations.Items, res) } }Also update references on lines 182, 190, and 196.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_change_commitments.go` around lines 163 - 179, The variables all_reservations and existing_reservations use snake_case; rename them to idiomatic Go camelCase (allReservations and existingReservations) wherever referenced in this function/method (including the List call, the loop over all_reservations.Items, append to existing_reservations.Items, and any subsequent uses such as where namePrefix is constructed and where existing_reservations is later read), update matching declarations and usages (e.g., replace var all_reservations v1alpha1.ReservationList and var existing_reservations v1alpha1.ReservationList and their subsequent references) so the code compiles with consistent camelCase identifiers.
172-179: Use strings.HasPrefix for prefix checking.The manual prefix check can be simplified using
strings.HasPrefixfor better readability.Proposed fix using strings.HasPrefix
// Filter by name prefix to find reservations for this commitment namePrefix := fmt.Sprintf("commitment-%s-", string(commitment.UUID)) var existingReservations v1alpha1.ReservationList for _, res := range allReservations.Items { - if len(res.Name) >= len(namePrefix) && res.Name[:len(namePrefix)] == namePrefix { + if strings.HasPrefix(res.Name, namePrefix) { existingReservations.Items = append(existingReservations.Items, res) } }As per coding guidelines, use
strings.Containsto check if a string is contained in another string (and similarlystrings.HasPrefixfor prefix checks).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_change_commitments.go` around lines 172 - 179, Replace the manual slice-based prefix check with strings.HasPrefix: in the loop that builds existing_reservations (variable namePrefix created from commitment.UUID and iterating over all_reservations.Items), call strings.HasPrefix(res.Name, namePrefix) instead of checking lengths and slicing; add the "strings" import if missing and keep appending matching reservations to existing_reservations.Items as before (reference symbols: namePrefix, existing_reservations, all_reservations, v1alpha1.ReservationList).internal/scheduling/reservations/controller/controller_test.go (1)
159-218: Consider extracting flavor group test data helper.The inline struct definition for flavor groups is verbose. While acceptable for test clarity, you could leverage the
createFlavorGroupKnowledgehelper pattern used insyncer_test.gofor consistency across test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/controller/controller_test.go` around lines 159 - 218, Extract the inline flavor-groups test data in controller_test.go into a reusable helper function (e.g., createFlavorGroupKnowledge) that constructs the flavorGroups payload, marshals it to JSON, wraps it into runtime.RawExtension and returns a *v1alpha1.Knowledge (or its Spec/Status) ready for tests; replace the inline block that builds flavorGroupKnowledge with a call to that helper, preserving fields like Spec.Extractor.Name, Spec.SchedulingDomain, Status.Raw/RawLength and Conditions so other tests can reuse the same helper as in syncer_test.go.internal/scheduling/reservations/commitments/api_info.go (1)
57-117: Verify GetAllFlavorGroups behavior when called with nil CRD.The
GetAllFlavorGroupsis called withnilas the second parameter on line 61, but later on line 105,knowledge.Get(ctx)is called separately to fetch the CRD for versioning. This results in potentially fetching the Knowledge CRD twice. Consider fetching it once and reusing.Proposed refactor to avoid duplicate CRD fetch
func (api *HTTPAPI) buildServiceInfo(ctx context.Context, log logr.Logger) (liquid.ServiceInfo, error) { - // Get all flavor groups from Knowledge CRDs knowledge := &reservations.FlavorGroupKnowledgeClient{Client: api.client} - flavorGroups, err := knowledge.GetAllFlavorGroups(ctx, nil) + + // Fetch knowledge CRD once for both flavor groups and versioning + knowledgeCRD, err := knowledge.Get(ctx) + if err != nil { + return liquid.ServiceInfo{ + Version: -1, + Resources: make(map[liquid.ResourceName]liquid.ResourceInfo), + }, err + } + + flavorGroups, err := knowledge.GetAllFlavorGroups(ctx, knowledgeCRD) if err != nil { - // Return -1 as version when knowledge is not ready return liquid.ServiceInfo{ Version: -1, Resources: make(map[liquid.ResourceName]liquid.ResourceInfo), }, err } // ... build resources map ... - // Get last content changed from flavor group knowledge and treat it as version var version int64 = -1 - if knowledgeCRD, err := knowledge.Get(ctx); err == nil && knowledgeCRD != nil && !knowledgeCRD.Status.LastContentChange.IsZero() { + if knowledgeCRD != nil && !knowledgeCRD.Status.LastContentChange.IsZero() { version = knowledgeCRD.Status.LastContentChange.Unix() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/api_info.go` around lines 57 - 117, buildServiceInfo currently calls knowledge.GetAllFlavorGroups(ctx, nil) then separately calls knowledge.Get(ctx), causing a duplicate CRD fetch; instead call knowledge.Get(ctx) first, handle error/nil as before (returning version -1 if knowledge not ready), then pass the obtained CRD to knowledge.GetAllFlavorGroups(ctx, crd) to build the resources and reuse the same CRD for computing version; update references to knowledge.GetAllFlavorGroups and knowledge.Get in buildServiceInfo accordingly and preserve existing error-return semantics and logging.internal/scheduling/reservations/commitments/capacity.go (2)
101-106: Consider logging when Knowledge CRD parsing fails.Silently continuing when
UnboxFeatureListfails could hide data issues. Adding a log statement would help with debugging.Proposed fix to add logging
// Parse features from Raw data features, err := v1alpha1.UnboxFeatureList[compute.HostDetails](knowledge.Status.Raw) if err != nil { - // Skip if we can't parse this knowledge + // Log and skip if we can't parse this knowledge + // Note: Consider using a logger passed as parameter for better testability continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/capacity.go` around lines 101 - 106, When UnboxFeatureList[compute.HostDetails] fails while parsing knowledge.Status.Raw in capacity.go, add a log statement instead of silently continuing: locate the block that calls v1alpha1.UnboxFeatureList[compute.HostDetails] and on err log the error and context (e.g., the CR name/UID or a snippet of knowledge.Status.Raw) via the package's existing logger (e.g., logger, processLogger, or the surrounding context's logger) before continue-ing so parsing failures are visible during debugging.
60-84: Placeholder implementation noted - capacity and usage values are unset.The TODO comments on lines 78-79 indicate that actual capacity and usage calculations are deferred. This is acceptable for an initial implementation, but ensure these TODOs are tracked for follow-up.
Do you want me to open a new issue to track the implementation of actual capacity and usage calculations?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/scheduling/reservations/commitments/capacity.go` around lines 60 - 84, The calculateAZCapacity placeholder leaves AZResourceCapacityReport Capacity/Usage unset; create a tracked issue that describes implementing capacity calculation from Reservation CRDs/host resources and usage from VM allocations, reference the calculateAZCapacity function and the TODOs inside it, then update the TODO comments in calculateAZCapacity (and any mention of AZResourceCapacityReport) to include the new issue ID and clear acceptance criteria (how to compute capacity, sources like Reservation CRDs and host details, expected optional semantics), so future work can link back and the code documents the tracking ticket.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 45: Update the wording/typos in AGENTS.md by replacing the phrase "struct
based" with "struct-based", change any occurrence of "miscallaneous" to
"miscellaneous", and capitalize "markdown" to "Markdown" (apply these edits
where the comment notes lines around the instances such as the sentence "If
applicable, use struct based test cases, but limit yourself to the most relevant
cases" and the other occurrences referenced). Ensure hyphenation and
capitalization are consistent across the file for these three terms.
In `@helm/bundles/cortex-nova/values.yaml`:
- Line 117: The values.yaml currently allows enabling reservations-controller
independently even though it relies on novaExternalScheduler provided by
nova-pipeline-controllers; add a runtime validation in main.go that checks the
chart/values configuration and fails fast if reservations-controller is enabled
while nova-pipeline-controllers is disabled (check the flags/struct fields that
map to "reservations-controller" and "nova-pipeline-controllers" and the
novaExternalScheduler URL) OR, alternatively, update values.yaml to document the
dependency clearly by adding a comment next to reservations-controller and
novaExternalScheduler that nova-pipeline-controllers must be enabled together
and list valid controller combinations; prefer adding the validation in main.go
to prevent misconfiguration.
In `@internal/knowledge/extractor/plugins/compute/flavor_groups.sql`:
- Around line 15-16: The WHERE clause is using fragile string LIKE on
extra_specs JSON; replace it with proper jsonb extraction e.g. parse extra_specs
as jsonb and test the hypervisor type via jsonb operators: use
(extra_specs::jsonb -> 'capabilities' ->> 'hypervisor_type') IN ('qemu','ch')
(or compare LOWER(...) if values may vary in case) instead of the LIKE patterns
so the query reliably filters on hypervisor type.
In `@internal/scheduling/nova/plugins/weighers/kvm_binpack_test.go`:
- Around line 100-137: Remove the duplicated table test entry that repeats the
"zero weights should raise error" case; locate the duplicate by the test case
name "zero weights should raise error" in the KVMBinpackStepOpts table (the
second occurrence) and delete that entire test struct so the table contains only
one "zero weights should raise error" case and the other unique cases remain
unchanged.
In `@internal/scheduling/reservations/commitments/api_change_commitments.go`:
- Around line 98-99: The function processCommitmentChanges currently uses
context.Background(); change its signature to accept a context.Context (e.g.,
processCommitmentChanges(ctx context.Context, w http.ResponseWriter, log
logr.Logger, req liquid.CommitmentChangeRequest, resp
*liquid.CommitmentChangeResponse) error) and replace context.Background() with
the passed ctx so cancellations propagate; then update the caller
HandleChangeCommitments to pass r.Context() into processCommitmentChanges when
invoking it. Ensure any downstream calls inside processCommitmentChanges that
create derived contexts use the passed ctx as parent.
In `@internal/scheduling/reservations/commitments/api_report_capacity.go`:
- Around line 37-40: The JSON decoder currently swallows all errors when
decoding into req (liquid.ServiceCapacityRequest); change the error handling
after json.NewDecoder(r.Body).Decode(&req) to only tolerate io.EOF as an empty
body and treat any other error as a client error by returning HTTP 400 Bad
Request. Specifically, update the handler that decodes the request so that if
err == io.EOF you set req = liquid.ServiceCapacityRequest{}; otherwise write a
400 response (e.g., http.Error or w.WriteHeader with an explanatory message) and
stop processing; ensure you import io for io.EOF and reference the same
json.NewDecoder(...).Decode(&req) call and the liquid.ServiceCapacityRequest
type.
In `@internal/scheduling/reservations/commitments/reservation_manager_test.go`:
- Around line 31-34: The test helper testFlavorGroup() is defined in
state_test.go but used by reservation_manager_test.go; move (or copy) the
testFlavorGroup() function into reservation_manager_test.go so the helper lives
with the tests that use it, and update any imports or dependencies inside
testFlavorGroup() to match reservation_manager_test.go (ensuring the signature
and return type compute.FlavorGroupFeature remain unchanged).
In `@internal/scheduling/reservations/commitments/reservation_manager.go`:
- Around line 107-113: The code appends to
touchedReservations/removedReservations before the API calls complete, causing
callers to see phantom changes on API errors; update the logic in the
reservation loop so you only append to removedReservations or
touchedReservations after the corresponding call succeeds (i.e., move the
removedReservations = append(...) and touchedReservations = append(...) to
immediately after a successful m.Delete(ctx, &res) or successful m.Create(ctx,
newRes) return), and ensure error returns (from methods like m.Delete and
m.Create) happen before any mutation of those slices; adjust the blocks around
m.Delete and m.Create in the functions handling reservations so the append
happens only on success.
- Around line 96-117: The loop over existing reservations dereferences
res.Spec.CommittedResourceReservation without a nil check and will panic for
malformed objects; update the loop in reservation_manager.go to first check if
res.Spec.CommittedResourceReservation == nil and treat such reservations as
invalid: log a clear message (include res.Name and commitmentUUID), append the
reservation to removedReservations, add its memory to deltaMemoryBytes, attempt
m.Delete(ctx, &res) and handle/delete errors the same way as the current
mismatch branch; otherwise continue with the existing flavor-group and projectID
comparisons and append to validReservations when appropriate.
- Around line 208-229: The current patch logic only updates reservation.Spec
when state fields are non-empty/non-nil, so removals (clearing AZ or clearing
StartTime/EndTime) never propagate; update the comparison and patch application
to detect differences even when state.AvailabilityZone is "" or
state.StartTime/state.EndTime is nil and then write the desired value (including
empty string or nil) into reservation.Spec: set
reservation.Spec.AvailabilityZone = state.AvailabilityZone unconditionally when
it differs, set reservation.Spec.StartTime = nil when state.StartTime == nil
otherwise set to &metav1.Time{Time: *state.StartTime}, and likewise set
reservation.Spec.EndTime = nil when state.EndTime == nil otherwise set to
&metav1.Time{Time: *state.EndTime}; keep using the existing patch =
client.MergeFrom(reservation.DeepCopy()) and log context (reservation,
availabilityZone, startTime, endTime).
In `@internal/scheduling/reservations/commitments/state.go`:
- Around line 153-199: FromReservations currently seeds metadata from
reservations[0], which is unsafe; update FromReservations (the code building
CommitmentState/using extractCommitmentUUID and stateLog) to first validate and
collect the consistent subset of reservations: iterate all reservations, skip
those with nil Spec.CommittedResourceReservation or a different
extractCommitmentUUID(res.Name), and collect candidates keyed by ProjectID,
DomainID, ResourceGroup (FlavorGroupName), AvailabilityZone, StartTime/EndTime;
choose canonical metadata by selecting the most common/consistent values among
candidates (or fail if none), then construct CommitmentState from those
canonical values and sum TotalMemoryBytes only from reservations that match the
canonical metadata (logging and ignoring inconsistent ones via stateLog as
before); ensure you return an error if no valid reservations remain.
In `@internal/scheduling/reservations/commitments/utils_test.go`:
- Around line 79-84: Add a regression test to
TestExtractCommitmentUUID_NoSlotIndex that covers dashed UUIDs: call
extractCommitmentUUID with "commitment-550e8400-e29b-41d4-a716-446655440000" and
assert it returns the full dashed UUID unchanged
("550e8400-e29b-41d4-a716-446655440000"); update the test function (or add a new
test function) to include this assertion so extractCommitmentUUID is validated
for UUIDs containing dashes and no slot index.
In `@internal/scheduling/reservations/commitments/utils.go`:
- Around line 17-24: GetMaxSlotIndex is currently treating any resource name
with a numeric last token as a commitment slot; restrict parsing to commitment
names only by ensuring the name begins with the "commitment-" prefix (or that
the first token equals "commitment") before splitting/parsing the last token.
Update the block that uses parts := strings.Split(res.Name, "-") and the
subsequent strconv.Atoi call to first check strings.HasPrefix(res.Name,
"commitment-") (or parts[0] == "commitment") and only then parse the trailing
numeric index and compare to maxIndex.
- Around line 36-45: The extractCommitmentUUID function currently splits
withoutPrefix and always strips the last dash-delimited token; change it to only
remove the final token when that token is a numeric slot index: after computing
parts := strings.Split(withoutPrefix, "-"), check the last token
(parts[len(parts)-1]) for being an integer (e.g., via strconv.Atoi or
strconv.ParseInt) and only rejoin parts[:len(parts)-1] when that parse succeeds;
otherwise return withoutPrefix unchanged—update extractCommitmentUUID
accordingly.
In `@internal/scheduling/reservations/controller/controller.go`:
- Around line 357-360: The early return in reconcileAllocations() when
len(res.Spec.CommittedResourceReservation.Allocations) == 0 prevents the status
patch and leaves stale entries in
res.Status.CommittedResourceReservation.Allocations; modify
reconcileAllocations() to continue and explicitly clear or set
res.Status.CommittedResourceReservation.Allocations to an empty map/slice (or
nil) and invoke the existing status-patch path so the cleared status is
persisted when the Spec allocation set transitions from non-empty to empty;
ensure you reference and update the same
Status.CommittedResourceReservation.Allocations field and reuse the controller's
existing status patch/update logic rather than simply returning early.
- Around line 58-59: The controller currently treats a nil r.DB as an error and
calls reconcileAllocations() for every Ready reservation even when no
DatabaseSecretRef is provided; change the reconcile path so that if
DatabaseSecretRef is nil (i.e., DB integration disabled) you skip allocation
verification and do not error on a nil r.DB. Concretely: in the reconcile flow
that initializes r.DB (referencing DatabaseSecretRef and the DB init block),
only attempt DB initialization and return an error if DatabaseSecretRef is
present and DB init fails; likewise, guard calls to reconcileAllocations() so
they run only when DatabaseSecretRef != nil (or r.DB != nil after init), leaving
pre-allocated reservations untouched when DB creds are absent.
In `@internal/scheduling/reservations/flavor_groups.go`:
- Around line 32-34: The Get() implementation in flavor_groups.go currently
treats a Kubernetes NotFound error as a hard failure; change it to normalize
NotFound to the "not ready" semantics by returning nil, nil when
apierrors.IsNotFound(err) (or equivalent k8s NotFound check) is true, while
preserving the existing fmt.Errorf(...) return for other errors; this ensures
callers like GetAllFlavorGroups and syncer.go can rely on knowledgeCRD == nil to
indicate not-ready instead of receiving an error.
In `@tools/spawner/cli/cli.go`:
- Around line 105-108: The formatting callback f inside ChooseServerGroup uses
sg.ID[:5] which can panic on short or empty IDs; modify f to safely truncate
sg.ID by checking its length (e.g., compute n := min(len(sg.ID), 5) or use a
shortID helper) and use sg.ID[:n] (or a conditional fallback like sg.ID) in the
fmt.Sprintf call so malformed IDs cannot cause a slice out-of-range panic.
In `@tools/spawner/main.go`:
- Around line 423-444: Pre-filter getServerGroupsResponse.ServerGroups to only
include entries where strings.HasPrefix(sg.Name, prefix), then use that filtered
slice for the prompt, the deletion loop (spawn goroutines via wg.Go) and for the
final log message; if the filtered slice is empty skip prompting and deletion
entirely or print a message stating no server groups matched the prefix; update
uses of sg.Name, sg.ID, wg, and projectCompute.Delete to operate on the filtered
list so the prompt and "Deleted ..." message accurately reflect what was
actually deleted.
---
Outside diff comments:
In `@helm/dev/cortex-prometheus-operator/Chart.yaml`:
- Around line 9-13: The Chart.yaml dependencies block is missing the required
local chart source comment; edit the dependencies section in Chart.yaml near the
kube-prometheus-stack entry (the dependency with name: kube-prometheus-stack,
repository: oci://ghcr.io/prometheus-community/charts, version: 82.10.3) and add
the mandatory comment line "# from: file://../../library/cortex-postgres"
directly above or next to that dependency declaration so the file includes the
required local chart path marker.
In `@internal/scheduling/reservations/controller/controller.go`:
- Around line 134-152: The merge base is taken after mutating res.Status which
makes client.MergeFrom(old) a no-op; call res.DeepCopy() to capture old before
modifying res.Status (use old := res.DeepCopy(); patch :=
client.MergeFrom(old)), then set res.Status.Host = res.Spec.TargetHost and call
r.Status().Patch(ctx, &res, patch) as before, preserving the existing error
handling for client.IgnoreNotFound(err).
- Around line 183-199: The memory and CPU conversions use the wrong scaling:
change memory conversion in the block that sets memoryMB (reading from
res.Spec.Resources["memory"]) to use memory.ScaledValue(resource.Mebi) so you
get MiB (8192 for 8GiB), and change CPU conversion (reading from
res.Spec.Resources["cpu"]) to compute whole vCPUs by taking
cpuQuantity.ScaledValue(resource.Milli) and dividing by 1000 (e.g., vcpus :=
uint64(cpuValue / 1000)); apply the same fixes to the other conversion block
later (lines ~251-259) so values match how reservation_manager.go stores bytes
and whole CPUs and validate the resulting integers before returning.
---
Nitpick comments:
In `@internal/knowledge/extractor/controller.go`:
- Line 212: Replace the use of the older interface{} type with the modern alias
any for the JSON comparison variables: update the declaration of oldData and
newData (currently declared as interface{}) to use any so the code conforms to
repo conventions and Go idioms.
In `@internal/scheduling/lib/detector_pipeline.go`:
- Around line 82-83: The mutex currently held by lock.Lock() with defer
lock.Unlock() spans too much work (including logging); narrow the critical
section by replacing defer unlock with an explicit lock.Unlock() immediately
after the shared-state writes so logging happens outside the lock. Locate the
lock usage (lock.Lock()/defer lock.Unlock()) in detector_pipeline.go (the same
pattern around lines 82–83 and 85–90), perform the shared-data updates while
locked, call lock.Unlock(), then do any logging or non-shared work outside the
lock to reduce contention.
In `@internal/scheduling/reservations/commitments/api_change_commitments_test.go`:
- Around line 113-117: Replace the bytes.Contains calls that check the response
text with strings.Contains since bodyStr is already a string: update the two
assertions that currently call bytes.Contains([]byte(bodyStr), []byte("...")) to
strings.Contains(bodyStr, "...") (referencing the bodyStr variable and the two
checks for "Version mismatch" and "12345"); also add "strings" to the test
imports and remove "bytes" if it becomes unused.
- Around line 24-223: Tests TestHandleChangeCommitments_VersionMismatch,
TestHandleChangeCommitments_DryRun and
TestProcessCommitmentChanges_KnowledgeNotReady repeat setup and request
boilerplate; refactor them into a single table-driven test that iterates cases
(e.g., name, initial Knowledge object or nil, request JSON, expected HTTP
status, expected content-type, expected response fields) and calls
HTTPAPI.HandleChangeCommitments for each row; extract common setup into reusable
helpers (scheme creation with v1alpha1.AddToScheme, fake client builder,
building API struct) and reuse the same httptest request/recorder flow inside
the loop, using unique symbols TestHandleChangeCommitments_Table (or consolidate
into TestHandleChangeCommitments) and keep assertions per-case driven by fields
in the table to preserve the existing checks for version mismatch, dry-run
rejection and "caches not ready" responses.
In `@internal/scheduling/reservations/commitments/api_change_commitments.go`:
- Line 225: The variable name time_start violates Go camelCase conventions;
rename the variable time_start to timeStart where it is declared (e.g., in the
block containing the time_start := time.Now() statement) and update every
use/reference in the same function/scope (search for time_start) to the new
identifier timeStart so the code follows Go naming style and compiles cleanly.
- Around line 163-179: The variables all_reservations and existing_reservations
use snake_case; rename them to idiomatic Go camelCase (allReservations and
existingReservations) wherever referenced in this function/method (including the
List call, the loop over all_reservations.Items, append to
existing_reservations.Items, and any subsequent uses such as where namePrefix is
constructed and where existing_reservations is later read), update matching
declarations and usages (e.g., replace var all_reservations
v1alpha1.ReservationList and var existing_reservations v1alpha1.ReservationList
and their subsequent references) so the code compiles with consistent camelCase
identifiers.
- Around line 172-179: Replace the manual slice-based prefix check with
strings.HasPrefix: in the loop that builds existing_reservations (variable
namePrefix created from commitment.UUID and iterating over
all_reservations.Items), call strings.HasPrefix(res.Name, namePrefix) instead of
checking lengths and slicing; add the "strings" import if missing and keep
appending matching reservations to existing_reservations.Items as before
(reference symbols: namePrefix, existing_reservations, all_reservations,
v1alpha1.ReservationList).
In `@internal/scheduling/reservations/commitments/api_info.go`:
- Around line 57-117: buildServiceInfo currently calls
knowledge.GetAllFlavorGroups(ctx, nil) then separately calls knowledge.Get(ctx),
causing a duplicate CRD fetch; instead call knowledge.Get(ctx) first, handle
error/nil as before (returning version -1 if knowledge not ready), then pass the
obtained CRD to knowledge.GetAllFlavorGroups(ctx, crd) to build the resources
and reuse the same CRD for computing version; update references to
knowledge.GetAllFlavorGroups and knowledge.Get in buildServiceInfo accordingly
and preserve existing error-return semantics and logging.
In `@internal/scheduling/reservations/commitments/api_report_capacity_test.go`:
- Around line 205-206: Replace all occurrences of the old empty interface type
with the Go alias any in this test file: update test struct field types and all
feature payload declarations (e.g. the emptyFeatures variable and other
map[string]interface{} uses passed into v1alpha1.BoxFeatureList and similar
calls) to use map[string]any and any instead of interface{}, and ensure function
calls like v1alpha1.BoxFeatureList receive the updated map[string]any types.
In `@internal/scheduling/reservations/commitments/capacity.go`:
- Around line 101-106: When UnboxFeatureList[compute.HostDetails] fails while
parsing knowledge.Status.Raw in capacity.go, add a log statement instead of
silently continuing: locate the block that calls
v1alpha1.UnboxFeatureList[compute.HostDetails] and on err log the error and
context (e.g., the CR name/UID or a snippet of knowledge.Status.Raw) via the
package's existing logger (e.g., logger, processLogger, or the surrounding
context's logger) before continue-ing so parsing failures are visible during
debugging.
- Around line 60-84: The calculateAZCapacity placeholder leaves
AZResourceCapacityReport Capacity/Usage unset; create a tracked issue that
describes implementing capacity calculation from Reservation CRDs/host resources
and usage from VM allocations, reference the calculateAZCapacity function and
the TODOs inside it, then update the TODO comments in calculateAZCapacity (and
any mention of AZResourceCapacityReport) to include the new issue ID and clear
acceptance criteria (how to compute capacity, sources like Reservation CRDs and
host details, expected optional semantics), so future work can link back and the
code documents the tracking ticket.
In `@internal/scheduling/reservations/commitments/reservation_manager_test.go`:
- Around line 84-123: The test reservations in existingReservations include the
LabelReservationType label but omit Spec.Type; add the Type field in each
v1alpha1.ReservationSpec (e.g. set Spec.Type =
v1alpha1.ReservationTypeCommittedResource or the matching enum used with
LabelReservationType/ReservationTypeLabelCommittedResource) so the test objects
mirror real reservations; update both reservation entries in the
existingReservations slice (referencing v1alpha1.ReservationSpec and Spec.Type)
to include the correct type value.
In `@internal/scheduling/reservations/commitments/state_test.go`:
- Around line 66-253: Consolidate repetitive error-path tests into table-driven
cases: replace the separate TestFromCommitment_InvalidResourceName,
TestFromReservations_EmptyList, TestFromReservations_MixedCommitmentUUIDs,
TestFromReservations_NonCommittedResourceType, and
TestGetFlavorGroupNameFromResource_Invalid with one or two table-driven tests
that enumerate input name/payload and the function to call (FromCommitment,
FromReservations, getFlavorGroupNameFromResource) plus expected error; implement
a loop that runs each case, invokes the appropriate symbol (FromCommitment,
FromReservations, getFlavorGroupNameFromResource) with the given input, and
asserts an error was returned, keeping the existing helper inputs (e.g.,
commitment structs and reservation slices) as inline case fields for clarity.
In `@internal/scheduling/reservations/commitments/syncer.go`:
- Around line 196-214: The deletion loop in the syncer currently returns an
error if s.Delete(ctx, &existing) fails, which should ignore NotFound errors;
update the block that calls s.Delete (inside the loop iterating
existingReservations.Items and using extractCommitmentUUID and
activeCommitments) to treat apierrors.IsNotFound(err) as a non-fatal condition
(log at info/debug and continue) and only return for other errors, and add the
import alias apierrors "k8s.io/apimachinery/pkg/api/errors".
In `@internal/scheduling/reservations/controller/controller_test.go`:
- Around line 159-218: Extract the inline flavor-groups test data in
controller_test.go into a reusable helper function (e.g.,
createFlavorGroupKnowledge) that constructs the flavorGroups payload, marshals
it to JSON, wraps it into runtime.RawExtension and returns a *v1alpha1.Knowledge
(or its Spec/Status) ready for tests; replace the inline block that builds
flavorGroupKnowledge with a call to that helper, preserving fields like
Spec.Extractor.Name, Spec.SchedulingDomain, Status.Raw/RawLength and Conditions
so other tests can reuse the same helper as in syncer_test.go.
In `@tools/plutono/provisioning/dashboards/cortex-status.json`:
- Around line 2321-2330: The "Triggered migrations" panel is querying the same
underlying metric cortex_detector_pipeline_run_duration_seconds_count (using
delta()) as the "Deschedulings" panel (which uses rate()), which may be
redundant and the title may be inaccurate; inspect the two panels (the one
titled "Triggered migrations" with expr=sum by (error)
(delta(cortex_detector_pipeline_run_duration_seconds_count{}[2m])) and the
"Deschedulings" panel that uses
rate(cortex_detector_pipeline_run_duration_seconds_count)) and either rename
"Triggered migrations" to reflect it shows absolute count delta, change its
query to a different metric/aggregation that actually represents migrations, or
consolidate the two panels into a single panel that presents both rate and
absolute change (or remove one) so the dashboard section is not duplicating the
same underlying signal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be05fcb0-f1c3-4185-8275-25271ea923a2
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtools/visualizer/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (85)
.coderabbit.yaml.github/workflows/push-charts.yaml.github/workflows/push-images.yaml.gitignoreAGENTS.mdCODEOWNERSTiltfileapi/v1alpha1/knowledge_types.goapi/v1alpha1/reservation_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/main.gogo.modhelm/bundles/cortex-cinder/Chart.yamlhelm/bundles/cortex-cinder/values.yamlhelm/bundles/cortex-crds/values.yamlhelm/bundles/cortex-ironcore/values.yamlhelm/bundles/cortex-manila/Chart.yamlhelm/bundles/cortex-manila/values.yamlhelm/bundles/cortex-nova/Chart.yamlhelm/bundles/cortex-nova/alerts/nova.alerts.yamlhelm/bundles/cortex-nova/templates/knowledges_kvm.yamlhelm/bundles/cortex-nova/templates/pipelines_kvm.yamlhelm/bundles/cortex-nova/values.yamlhelm/bundles/cortex-pods/values.yamlhelm/dev/cortex-prometheus-operator/Chart.yamlhelm/library/cortex-postgres/Chart.yamlhelm/library/cortex/Chart.yamlhelm/library/cortex/files/crds/cortex.cloud_knowledges.yamlhelm/library/cortex/files/crds/cortex.cloud_reservations.yamlhelm/library/cortex/values.yamlinternal/knowledge/extractor/controller.gointernal/knowledge/extractor/plugins/compute/flavor_groups.gointernal/knowledge/extractor/plugins/compute/flavor_groups.sqlinternal/knowledge/extractor/plugins/compute/flavor_groups_test.gointernal/knowledge/extractor/supported_extractors.gointernal/knowledge/kpis/plugins/compute/resource_capacity_kvm_test.gointernal/scheduling/lib/detector_monitor.gointernal/scheduling/lib/detector_pipeline.gointernal/scheduling/lib/detector_pipeline_test.gointernal/scheduling/nova/integration_test.gointernal/scheduling/nova/plugins/filters/filter_has_enough_capacity.gointernal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.gointernal/scheduling/nova/plugins/filters/filter_packed_virtqueue.gointernal/scheduling/nova/plugins/filters/filter_packed_virtqueue_test.gointernal/scheduling/nova/plugins/weighers/kvm_binpack.gointernal/scheduling/nova/plugins/weighers/kvm_binpack_test.gointernal/scheduling/nova/plugins/weighers/kvm_failover_evacuation_test.gointernal/scheduling/nova/plugins/weighers/kvm_prefer_smaller_hosts.gointernal/scheduling/nova/plugins/weighers/kvm_prefer_smaller_hosts_test.gointernal/scheduling/reservations/commitments/api.gointernal/scheduling/reservations/commitments/api_change_commitments.gointernal/scheduling/reservations/commitments/api_change_commitments_test.gointernal/scheduling/reservations/commitments/api_info.gointernal/scheduling/reservations/commitments/api_info_test.gointernal/scheduling/reservations/commitments/api_report_capacity.gointernal/scheduling/reservations/commitments/api_report_capacity_test.gointernal/scheduling/reservations/commitments/capacity.gointernal/scheduling/reservations/commitments/client.gointernal/scheduling/reservations/commitments/client_test.gointernal/scheduling/reservations/commitments/reservation_manager.gointernal/scheduling/reservations/commitments/reservation_manager_test.gointernal/scheduling/reservations/commitments/state.gointernal/scheduling/reservations/commitments/state_test.gointernal/scheduling/reservations/commitments/syncer.gointernal/scheduling/reservations/commitments/syncer_test.gointernal/scheduling/reservations/commitments/utils.gointernal/scheduling/reservations/commitments/utils_test.gointernal/scheduling/reservations/controller/client.gointernal/scheduling/reservations/controller/client_test.gointernal/scheduling/reservations/controller/controller.gointernal/scheduling/reservations/controller/controller_test.gointernal/scheduling/reservations/controller/monitor.gointernal/scheduling/reservations/controller/monitor_test.gointernal/scheduling/reservations/flavor_groups.gopostgres/Dockerfiletools/plutono/provisioning/dashboards/cortex-status.jsontools/spawner/cli/cli.gotools/spawner/main.gotools/spawner/types/server_group.gotools/visualizer/Dockerfiletools/visualizer/app.yamltools/visualizer/nginx.conftools/visualizer/nova.htmltools/visualizer/role.yamltools/visualizer/shared.css
💤 Files with no reviewable changes (14)
- internal/scheduling/reservations/controller/client_test.go
- internal/scheduling/nova/plugins/filters/filter_packed_virtqueue_test.go
- Tiltfile
- helm/library/cortex/values.yaml
- internal/scheduling/nova/plugins/filters/filter_packed_virtqueue.go
- internal/scheduling/reservations/controller/client.go
- internal/scheduling/reservations/commitments/client_test.go
- tools/visualizer/role.yaml
- tools/visualizer/nova.html
- tools/visualizer/Dockerfile
- tools/visualizer/nginx.conf
- tools/visualizer/shared.css
- helm/bundles/cortex-nova/templates/pipelines_kvm.yaml
- tools/visualizer/app.yaml
This alert has been around for some time, and so far never reported false positives or flapped. So we can consider it stable. Since we're going onto the critical path with cortex for nova kvm, it's crucial that we escalate this alert. There's also an actionable playbook for this alert already.
In cobaltcore-dev/openstack-hypervisor-operator#257 we introduced a new `effectiveCapacity` field of the hypervisor crd. In cobaltcore-dev/kvm-node-agent#70 we populate this field. Now we can upgrade cortex so it performs scheduling on the overcommitted capacity.
This change adds a controller to the nova scheduling controller manager, which finds hypervisors with specific traits and assigns desired overcommit ratios to them.
## Changes - Adjusted the kvm resource capacity kpi to use the new `EffectiveCapacity` from the hypervisor CRD, see: cobaltcore-dev/openstack-hypervisor-operator#257
- fix hypervisor crd Tilt reference - make test with proper summary based on gotestsum - Adding integration tests for commitment change API - Commitment change API handles more corner cases - commitment config added - moving endpoint to "/v1/commitments/..."
This change adds the filter_correct_az to the pipelines we're actively using for nova vms outside an experimental stage. In this way we stay consistent to what we have deployed, slowly phasing in the nova filters.
PhilippMatthes
left a comment
There was a problem hiding this comment.
Re-approve on the latest changes
Test Coverage ReportTest Coverage 📊: 69.6% |
Uh oh!
There was an error while loading. Please reload this page.