Open
Conversation
Add make install-all-oci target that deploys HyperFleet on Oracle Kubernetes Engine with in-cluster RabbitMQ, quay.io v0.2.0 images, and adapter1. GCP-specific PodMonitoring CRDs are disabled via a sentinel values-oci.yaml overlay. Pin sentinel chart to v0.2.0 tag for OCI deployments to match the v0.2.0 binary config format.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds OCI/OKE-focused Helm values and two new adapter charts plus Makefile targets to install/uninstall OCI-specific deployments. Makefile: adds Sequence Diagram(s)sequenceDiagram
participant Broker as Broker (RabbitMQ)
participant Adapter as Adapter-hypershift / Adapter-nodepool
participant API as Hyperfleet API
participant HS as HyperShift Management Cluster (k8s API)
Broker->>Adapter: Deliver event (clusterId/nodepoolId, generation, params)
Adapter->>API: GET /clusters/{clusterId} or GET /nodepools/{nodepoolId}
API-->>Adapter: Resource details (name, spec, conditions)
Adapter->>Adapter: Evaluate preconditions (notReady && clusterAvailable)
alt preconditions pass
Adapter->>HS: Apply Namespace / HostedCluster or NodePool manifest
HS-->>Adapter: Resource created/updated + status
end
Adapter->>API: POST /clusters/{clusterId}/statuses or /nodepools/{nodepoolId}/statuses (status payload)
API-->>Adapter: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
280-293: Add OCI config path to dry-run Helm validation.The new OCI flow (
SENTINEL_CHART_REF=v0.2.0+values-oci.yaml) is not explicitly exercised by current dry-run validation, so schema/value drift can slip through CI.Suggested follow-up diff
.PHONY: validate-helm-charts validate-helm-charts: check-helm ## Render all Helm charts with helm template (no cluster required) @@ $(call validate-chart,sentinel-clusters,$(SENTINEL_CHART_REF),\ --set hyperfleet-sentinel.broker.type=$(BROKER_TYPE) \ $(if $(REGISTRY),--set hyperfleet-sentinel.image.registry=$(REGISTRY)) \ $(if $(SENTINEL_REPOSITORY),--set hyperfleet-sentinel.image.repository=$(SENTINEL_REPOSITORY)) \ - --set hyperfleet-sentinel.image.tag=$(SENTINEL_IMAGE_TAG)) + --set hyperfleet-sentinel.image.tag=$(SENTINEL_IMAGE_TAG) \ + $(SENTINEL_EXTRA_ARGS)) @@ .PHONY: ci-dry-run ci-dry-run: ci-validate ## Layer 2: Static + dry-run validation (no credentials required) $(MAKE) validate-helm-charts BROKER_TYPE=rabbitmq $(MAKE) validate-helm-charts BROKER_TYPE=googlepubsub + $(MAKE) validate-helm-charts \ + BROKER_TYPE=rabbitmq \ + SENTINEL_CHART_REF=v0.2.0 \ + SENTINEL_EXTRA_ARGS="--values $(HELM_DIR)/sentinel-clusters/values-oci.yaml"As per coding guidelines, "-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 280 - 293, The dry-run Helm validation is not including the OCI-specific values file used by install-all-oci; update the Helm dry-run/validation target to pass the OCI values path (the same file referenced by SENTINEL_EXTRA_ARGS / values-oci.yaml) and the OCI chart ref (SENTINEL_CHART_REF=v0.2.0) so the dry-run exercises the OCI flow; locate the Helm dry-run/validation recipe (the target that performs helm template/helm install --dry-run/--validate) and add the --values $(HELM_DIR)/sentinel-clusters/values-oci.yaml (or include $(SENTINEL_EXTRA_ARGS)) and ensure it uses $(SENTINEL_CHART_REF) when validating sentinel charts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/sentinel-clusters/values-oci.yaml`:
- Around line 13-18: The OCI overlay is hardcoding RabbitMQ credentials and
topic (broker.rabbitmq.url = amqp://guest:guest@rabbitmq:5672/ and broker.topic
= hyperfleet-clusters) which can override generated RABBITMQ_URL/namespace
settings; remove these hardcoded values from values-oci.yaml so the overlay does
not set broker.rabbitmq.url or broker.topic (or set them to
empty/null/placeholders) and allow the Makefile-generated RABBITMQ_URL and
namespace-derived broker settings to take precedence; change the broker block
(broker.type, broker.rabbitmq) to omit the url and topic keys or replace them
with non-overriding placeholders so non-default deployments aren’t broken.
---
Nitpick comments:
In `@Makefile`:
- Around line 280-293: The dry-run Helm validation is not including the
OCI-specific values file used by install-all-oci; update the Helm
dry-run/validation target to pass the OCI values path (the same file referenced
by SENTINEL_EXTRA_ARGS / values-oci.yaml) and the OCI chart ref
(SENTINEL_CHART_REF=v0.2.0) so the dry-run exercises the OCI flow; locate the
Helm dry-run/validation recipe (the target that performs helm template/helm
install --dry-run/--validate) and add the --values
$(HELM_DIR)/sentinel-clusters/values-oci.yaml (or include
$(SENTINEL_EXTRA_ARGS)) and ensure it uses $(SENTINEL_CHART_REF) when validating
sentinel charts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54a2d12b-ba43-49c3-bd6e-b33b7f0146ae
📒 Files selected for processing (2)
Makefilehelm/sentinel-clusters/values-oci.yaml
vkareh
requested review from
ciaranRoche and
rh-amarin
and removed request for
tirthct and
yasun1
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Add a new adapter helm package that creates HostedCluster resources on a remote HyperShift management cluster via a mounted kubeconfig secret. Uses NodePort for Ignition/OAuth services and parameterizes OCI region, compartment, release image, CPO image, and worker node IP via env vars.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/adapter-hypershift/adapter-task-config.yaml`:
- Around line 39-41: The workerNodeIP configuration (source env.WORKER_NODE_IP)
is currently optional but is always injected into nodePort.address causing
broken endpoints when empty; make WORKER_NODE_IP mandatory by updating the
adapter-task-config entry for workerNodeIP to require a non-empty value (e.g.,
add a required flag/validation for source env.WORKER_NODE_IP or enforce
presence) and ensure the template logic that sets nodePort.address (references
to nodePort.address and workerNodeIP) relies on that validated value so the
process fails early if WORKER_NODE_IP is unset.
- Around line 71-80: The task currently creates the Namespace
(clustersNamespace) but does not ensure the required Secrets (oci-credentials,
pull-secret, ssh-key) exist, leaving HostedCluster creation stuck; add a
preflight validation or creation step for those Secrets: either create manifests
for oci-credentials, pull-secret and ssh-key under the same task (or a new task
run before HostedCluster) or add a validation check that queries for each Secret
in the target namespace and fails fast with a clear error message listing any
missing Secret names (referencing clustersNamespace, HostedCluster, and the
secret names oci-credentials, pull-secret, ssh-key) so installation aborts
rather than hanging.
- Around line 57-61: The expression for the "clusterNotReady" rule assumes
status.conditions exists; guard it before filtering so a missing conditions
field falls back to true rather than erroring. Modify the expression in the
clusterNotReady rule to check existence/length of status.conditions (e.g.,
ensure status.conditions != null and size()>0) before calling filter, then apply
the existing Ready check and fallback to true; update the expression attached to
the name "clusterNotReady" to use this guarded form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1287e88a-4247-4494-baca-14470c7c3fba
📒 Files selected for processing (5)
helm/adapter-hypershift/Chart.yamlhelm/adapter-hypershift/adapter-config.yamlhelm/adapter-hypershift/adapter-task-config.yamlhelm/adapter-hypershift/charts/hyperfleet-adapter-2.0.0.tgzhelm/adapter-hypershift/values.yaml
✅ Files skipped from review due to trivial changes (3)
- helm/adapter-hypershift/Chart.yaml
- helm/adapter-hypershift/values.yaml
- helm/adapter-hypershift/adapter-config.yaml
Add new adapter-hypershift-nodepool Helm chart that subscribes to the nodepools topic, validates the parent HostedCluster is Available via adapter status, creates a HyperShift NodePool CR on the management cluster, and reports status back to the API. Fix adapter-hypershift task config to use Route instead of NodePort for Ignition/OAuth. Add values-oci.yaml for sentinel-nodepools with RabbitMQ config. Pin sentinel-nodepools Chart.yaml dependency to v0.2.0.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
helm/adapter-hypershift/adapter-task-config.yaml (2)
55-58:⚠️ Potential issue | 🟠 MajorGuard
status.conditionsbefore filtering to prevent precondition failures.Line 56 assumes
status.conditionsexists. For fresh cluster objects, this can be absent and crash precondition evaluation instead of falling back totrue.Suggested fix
- name: "clusterNotReady" expression: | - status.conditions.filter(c, c.type == "Ready").size() > 0 - ? status.conditions.filter(c, c.type == "Ready")[0].status != "True" - : true + !has(status) || !has(status.conditions) + ? true + : (status.conditions.filter(c, c.type == "Ready").size() > 0 + ? status.conditions.filter(c, c.type == "Ready")[0].status != "True" + : true)As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/adapter-hypershift/adapter-task-config.yaml` around lines 55 - 58, The current readiness expression assumes status.conditions exists and may fail for new objects; update the expression to first guard that status.conditions is present and has entries before filtering (e.g., check status.conditions != null and/or status.conditions.size() > 0), then perform the filter for c.type == "Ready" and test its [0].status, otherwise return true; refer to the existing status.conditions and the Ready condition filter in your change and ensure the fallback path returns true when conditions are absent.
67-78:⚠️ Potential issue | 🟠 MajorFail fast when required Secrets are missing (or create them in-task).
Lines 101/106/108 reference
oci-credentials,pull-secret, andssh-key, but Lines 67-78 only ensure the namespace exists. On a fresh OCI path, this can leave HostedCluster provisioning stuck/failing late. Add a preflight validation step that checks these Secrets and aborts with a clear error listing missing names (or create them explicitly before HostedCluster).As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 101-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/adapter-hypershift/adapter-task-config.yaml` around lines 67 - 78, The task currently only ensures the Namespace via the clustersNamespace manifest but does not validate or create required Secrets (oci-credentials, pull-secret, ssh-key), which can cause HostedCluster provisioning to fail later; add a preflight step (before clustersNamespace or as a separate transport/manifest block) that checks for the existence of those three Secrets in the target namespace and fails fast with a clear error listing any missing names (or create them explicitly) so that the HostedCluster bootstrap (referenced later lines using those names) does not hang or error unexpectedly; ensure the preflight uses the same kubernetes client/transport and returns a non-zero failure when any required Secret is missing.
🧹 Nitpick comments (1)
helm/adapter-hypershift-nodepool/values.yaml (1)
18-31: Broker type defaults togooglepubsubbut PR targets RabbitMQ for OCI.The PR description states OCI deployment uses "in-cluster RabbitMQ," but
broker.typedefaults togooglepubsub. Users deploying to OCI will need to override this. Consider either changing the default or adding a comment noting OCI deployments should setbroker.type: rabbitmq.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/adapter-hypershift-nodepool/values.yaml` around lines 18 - 31, The values.yaml currently defaults broker.type to googlepubsub which contradicts the PR (OCI uses in-cluster RabbitMQ); update values.yaml so broker.type defaults to rabbitmq or, if you prefer not to change the default, add a clear comment above the broker.type entry instructing OCI deployments to set broker.type: rabbitmq and to populate the rabbitmq.* fields (url/queue/exchange/routingKey); ensure references to googlepubsub (projectId/subscriptionId/topic) remain but are marked optional for non-GCP setups and verify the keys broker.type, googlepubsub, and rabbitmq are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/adapter-hypershift-nodepool/adapter-task-config.yaml`:
- Around line 15-18: The parameter "generation" is defined twice (as source:
"event.generation" and again when captured from the API in preconditions),
causing the API-captured value to shadow the event value; decide which source
must drive observed_generation and either (A) rename the precondition-captured
variable (e.g., api_generation) and use that explicit name where you need the
API value, or (B) keep the event-derived variable and stop overwriting it by
removing/renaming the capture in the precondition; ensure the status payload
uses the intended variable name (e.g., observed_generation should be set from
event.generation if you want the event value, or from api_generation if you
intend to prefer the API) and update all references in the template (variable
"generation", source "event.generation", precondition capture) accordingly to
eliminate ambiguous shadowing.
- Around line 119-136: The NodePool spec in adapter-task-config.yaml is using
hardcoded values (replicas: 2, ocpus: 4, memoryInGBs: 16, bootVolumeSize: 120,
etc.) instead of the captured nodepoolSpec; update the template to reference
.nodepoolSpec for those fields (use the same pattern as nodepool-manifest.yaml,
e.g. index .nodepoolSpec "replicas" | default 2) so replicas,
platform.oci.instanceShapeConfig.ocpus,
platform.oci.instanceShapeConfig.memoryInGBs, platform.oci.bootVolumeSize (and
any image override under release.image if applicable) read from .nodepoolSpec
with sensible defaults.
In `@helm/adapter-hypershift-nodepool/nodepool-manifest.yaml`:
- Around line 19-28: The YAML has nested double quotes inside Helm template
expressions which break parsing; fix by replacing the inner double quotes around
nodepoolSpec keys with single quotes (e.g., change index .nodepoolSpec
"instanceShape" to index .nodepoolSpec 'instanceShape') or by escaping the inner
quotes, and ensure string values like availabilityDomain, subnetId and
release.image use single-quoted YAML scalars while numeric fields
(bootVolumeSize, ocpus, memoryInGBs) remain unquoted; update the expressions for
instanceShape, instanceShapeConfig.ocpus, instanceShapeConfig.memoryInGBs,
availabilityDomain, subnetId, bootVolumeSize and release.image accordingly so
the template parses correctly.
In `@helm/sentinel-nodepools/values-oci.yaml`:
- Line 16: Remove the hardcoded credentials in the values file by replacing the
literal amqp://guest:guest@rabbitmq:5672/ value for the url key in
values-oci.yaml with a non-credential placeholder and wire the real broker URL
via a Kubernetes Secret; create a Secret containing the broker URL (or
username/password) and update the Deployment/StatefulSet spec that consumes the
values (where url is used) to read it via valueFrom.secretKeyRef (or env:
valueFrom.secretKeyRef), and ensure the guest account is not used in OCI by
referencing the secret key (e.g., rabbitmq-url or rabbitmq-credentials) at
runtime instead of baking credentials into the url value.
---
Duplicate comments:
In `@helm/adapter-hypershift/adapter-task-config.yaml`:
- Around line 55-58: The current readiness expression assumes status.conditions
exists and may fail for new objects; update the expression to first guard that
status.conditions is present and has entries before filtering (e.g., check
status.conditions != null and/or status.conditions.size() > 0), then perform the
filter for c.type == "Ready" and test its [0].status, otherwise return true;
refer to the existing status.conditions and the Ready condition filter in your
change and ensure the fallback path returns true when conditions are absent.
- Around line 67-78: The task currently only ensures the Namespace via the
clustersNamespace manifest but does not validate or create required Secrets
(oci-credentials, pull-secret, ssh-key), which can cause HostedCluster
provisioning to fail later; add a preflight step (before clustersNamespace or as
a separate transport/manifest block) that checks for the existence of those
three Secrets in the target namespace and fails fast with a clear error listing
any missing names (or create them explicitly) so that the HostedCluster
bootstrap (referenced later lines using those names) does not hang or error
unexpectedly; ensure the preflight uses the same kubernetes client/transport and
returns a non-zero failure when any required Secret is missing.
---
Nitpick comments:
In `@helm/adapter-hypershift-nodepool/values.yaml`:
- Around line 18-31: The values.yaml currently defaults broker.type to
googlepubsub which contradicts the PR (OCI uses in-cluster RabbitMQ); update
values.yaml so broker.type defaults to rabbitmq or, if you prefer not to change
the default, add a clear comment above the broker.type entry instructing OCI
deployments to set broker.type: rabbitmq and to populate the rabbitmq.* fields
(url/queue/exchange/routingKey); ensure references to googlepubsub
(projectId/subscriptionId/topic) remain but are marked optional for non-GCP
setups and verify the keys broker.type, googlepubsub, and rabbitmq are updated
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0db321d-4e80-477f-ab07-28d538dd728e
📒 Files selected for processing (9)
helm/adapter-hypershift-nodepool/Chart.yamlhelm/adapter-hypershift-nodepool/adapter-config.yamlhelm/adapter-hypershift-nodepool/adapter-task-config.yamlhelm/adapter-hypershift-nodepool/charts/hyperfleet-adapter-2.0.0.tgzhelm/adapter-hypershift-nodepool/nodepool-manifest.yamlhelm/adapter-hypershift-nodepool/values.yamlhelm/adapter-hypershift/adapter-task-config.yamlhelm/adapter-hypershift/charts/hyperfleet-adapter-2.0.0.tgzhelm/sentinel-nodepools/values-oci.yaml
✅ Files skipped from review due to trivial changes (2)
- helm/adapter-hypershift-nodepool/adapter-config.yaml
- helm/adapter-hypershift-nodepool/Chart.yaml
Add has() guards to clusterNotReady/nodepoolNotReady precondition expressions so fresh resources without status.conditions fall back to true instead of erroring. Replace hardcoded NodePool spec values in adapter-task-config with index .nodepoolSpec lookups matching the pattern in nodepool-manifest.yaml.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
helm/adapter-hypershift/adapter-task-config.yaml (1)
70-80:⚠️ Potential issue | 🟠 MajorFail fast when required secrets are absent in the target namespace.
The task ensures the namespace exists, but it neither creates nor validates
oci-credentials,pull-secret, andssh-keybefore creatingHostedCluster. This can leave installs stuck instead of failing with a clear prerequisite error.Also applies to: 103-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/adapter-hypershift/adapter-task-config.yaml` around lines 70 - 80, The clustersNamespace task (name "clustersNamespace") currently ensures the Namespace exists but does not check for required secrets; update the task to explicitly validate that the secrets named "oci-credentials", "pull-secret", and "ssh-key" exist in the target namespace and fail fast with a clear error before proceeding to create the HostedCluster resource; implement this by adding pre-create checks (e.g., a Kubernetes Secret get/list step) and return a clear error if any secret is missing so the pipeline aborts instead of hanging during HostedCluster creation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/adapter-hypershift-nodepool/adapter-task-config.yaml`:
- Around line 95-97: The expression for "clusterAvailable" assumes the first
adapter-hypershift item has a conditions field; guard that by checking the
conditions field exists and is an array (or has size>0) before calling .filter
on it. Update the expression used in clusterAvailable to first get the item
(items.filter(s, s.adapter == "adapter-hypershift")[0] as a variable concept)
and then evaluate conditions != null and conditions.size() > 0 (or conditions &&
conditions.size() > 0) before performing conditions.filter(...)[0].status ==
"True", returning false if conditions is missing.
- Around line 130-138: The YAML uses double-quoted strings that contain
templated index calls with inner double quotes (e.g., instanceShape,
instanceShapeConfig.ocpus/memoryInGBs, availabilityDomain, subnetId,
bootVolumeSize, and release.image), which breaks parsing; update those fields
(references to instanceShape, index .nodepoolSpec "ocpus", index .nodepoolSpec
"memoryInGBs", index .nodepoolSpec "availabilityDomain", index .nodepoolSpec
"subnetId", index .nodepoolSpec "bootVolumeSize", and index .nodepoolSpec
"releaseImage") to use single-quoted outer strings so the inner double quotes
are preserved for the template engine (i.e., change outer "..." to '...' around
the {{ index .nodepoolSpec "..." | default ... }} expressions).
---
Duplicate comments:
In `@helm/adapter-hypershift/adapter-task-config.yaml`:
- Around line 70-80: The clustersNamespace task (name "clustersNamespace")
currently ensures the Namespace exists but does not check for required secrets;
update the task to explicitly validate that the secrets named "oci-credentials",
"pull-secret", and "ssh-key" exist in the target namespace and fail fast with a
clear error before proceeding to create the HostedCluster resource; implement
this by adding pre-create checks (e.g., a Kubernetes Secret get/list step) and
return a clear error if any secret is missing so the pipeline aborts instead of
hanging during HostedCluster creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df540df5-5234-4169-a9fe-bab9e562e561
📒 Files selected for processing (2)
helm/adapter-hypershift-nodepool/adapter-task-config.yamlhelm/adapter-hypershift/adapter-task-config.yaml
…apter Add has() guard to clusterAvailable expression so missing conditions field returns false instead of erroring. Switch outer double quotes to single quotes on nodepoolSpec index expressions to avoid nested quote YAML parsing errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/adapter-hypershift-nodepool/adapter-task-config.yaml`:
- Around line 105-108: The current validationCheck expression (using
nodepoolNotReady && clusterAvailable) stops reconciliation once Ready=True,
preventing later changes to .nodepoolSpec from being applied; update the logic
so reconciliation continues when the HostedCluster is Available and either the
NodePool is not Ready OR the desired .nodepoolSpec has changed. Concretely:
modify the validationCheck expression to include a spec-changed condition (e.g.,
clusterAvailable && (nodepoolNotReady || nodepoolSpecChanged)) and implement
nodepoolSpecChanged (or similar) to detect differences between the live NodePool
spec and .nodepoolSpec (for example via a spec-hash/annotation or direct field
comparisons for replicas/shape/releaseImage) so subsequent updates (replicas,
shape, release image) will trigger reconciliation even after Ready=True.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aee02dc8-7902-4a60-bf03-38f1dd41ff65
📒 Files selected for processing (1)
helm/adapter-hypershift-nodepool/adapter-task-config.yaml
|
@vkareh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add make install-all-oci target that deploys HyperFleet on Oracle Kubernetes Engine with in-cluster RabbitMQ, quay.io v0.2.0 images, and adapter1. GCP-specific PodMonitoring CRDs are disabled via a sentinel values-oci.yaml overlay.
Pin sentinel chart to v0.2.0 tag for OCI deployments to match the v0.2.0 binary config format.
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)Summary by CodeRabbit
New Features
Chores