HYPERFLEET-572: Add cl-maestro adapter configs#34
Conversation
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughAdds three test configuration files under testdata/adapter-configs/cl-maestro/: an AdapterConfig (adapter-config.yaml) declaring adapter metadata, API/broker/broker and maestro client settings and optional TLS placeholders; an AdapterTaskConfig (adapter-task-config.yaml) that defines params, preconditions (clusterStatus GET and CEL validation), resource provisioning via a ManifestWork (Namespace + ConfigMap), nested discoveries, manifestConfigs, post payloads that compute conditions/observed fields from discovered resources, and a postAction to report cluster status; and a Helm values file (values.yaml) mapping and enabling these configs plus image, broker, and RBAC settings. Sequence Diagram(s)sequenceDiagram
participant AdapterTask as AdapterTask (cl-maestro)
participant HyperfleetAPI as Hyperfleet API
participant Spoke as Spoke Cluster (ManifestWork)
participant Discovery as Discovered Resources (Namespace/ConfigMap)
AdapterTask->>HyperfleetAPI: GET clusterStatus (precondition)
HyperfleetAPI-->>AdapterTask: clusterStatus payload
AdapterTask->>AdapterTask: evaluate CEL preconditions & validation
AdapterTask->>Spoke: Create/Apply ManifestWork (Namespace + ConfigMap)
Spoke-->>AdapterTask: ManifestWork status / discovery refs
AdapterTask->>Discovery: read Namespace and ConfigMap via discovery
Discovery-->>AdapterTask: resource details (phase, data, metadata)
AdapterTask->>HyperfleetAPI: POST /reportClusterStatus with statusPayload (conditions, observed_generation, data)
HyperfleetAPI-->>AdapterTask: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 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 |
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
testdata/adapter-configs/clusters-cl-maestro/values.yaml (1)
8-8: Pin the adapter image to an immutable version instead oflatest.Line 8 uses a mutable tag, which makes e2e runs non-deterministic over time. Prefer a fixed tag (or digest) and keep substitution in automation.
Proposed patch
- tag: latest + # make install should substitute this to a pinned version or digest + tag: CHANGE_THIS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/clusters-cl-maestro/values.yaml` at line 8, Replace the mutable image tag under the YAML key "tag" (currently set to "latest") with a pinned, immutable image identifier (e.g., a specific semver tag or image digest) and update automation to substitute that fixed value during e2e runs; ensure the change targets the "tag" field in this values.yaml so CI uses a deterministic image version rather than "latest".testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml (1)
132-133: Align the docs annotation with thecl-maestroadapter.Line 133 points to
aro-hcpdocs, which is inconsistent with this adapter config and can mislead troubleshooting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml` around lines 132 - 133, The documentation URL is incorrect for the cl-maestro adapter; update the documentation field (the "documentation" annotation) in the adapter-task-config.yaml entry for the cl-maestro adapter so it points to the correct cl-maestro docs (replace "https://docs.hyperfleet.io/adapters/aro-hcp" with the appropriate cl-maestro documentation URL or internal docs path), keeping the description value intact; ensure the string matches the canonical cl-maestro documentation location used across the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-config.yaml`:
- Around line 40-44: The sourceId value is too generic and may collide across
adapters; update the adapter's identifiers by replacing sourceId
"hyperfleet-adapter" and the clientId "hyperfleet-adapter-client" with
adapter-specific, unique values (for example including the adapter name or
environment suffix) to ensure routing/isolation; modify the entries for sourceId
and clientId in the adapter-config.yaml so they are globally unique for this
adapter and follow your project's naming convention.
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml`:
- Around line 27-30: The TODO indicates the source for the adapterName is
ambiguous; update the adapter task config so the "name: \"adapterName\"" entry
uses the canonical path "source: \"config.adapter.metadata.name\"" (replace the
current config.adapterConfig path), remove the TODO comment, and keep or adjust
the default "cl-maestro" as fallback; ensure the change is applied to the
adapterName block so any code reading adapterName will pull from
config.adapter.metadata.name.
- Around line 55-57: The placementClusterName field is hardcoded via the
expression "\"cluster1\"" so work is always routed to a fixed cluster; change
the expression for placementClusterName to reference the runtime/context
variable (e.g., the event or placement value provided by the adapter) instead of
a literal string and update the code that reads placementClusterName (the logic
that sends work at the place referenced in the diff around line 77) to consume
that dynamic field so the target maestro is selected from the event/context
rather than "cluster1".
---
Nitpick comments:
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml`:
- Around line 132-133: The documentation URL is incorrect for the cl-maestro
adapter; update the documentation field (the "documentation" annotation) in the
adapter-task-config.yaml entry for the cl-maestro adapter so it points to the
correct cl-maestro docs (replace "https://docs.hyperfleet.io/adapters/aro-hcp"
with the appropriate cl-maestro documentation URL or internal docs path),
keeping the description value intact; ensure the string matches the canonical
cl-maestro documentation location used across the repo.
In `@testdata/adapter-configs/clusters-cl-maestro/values.yaml`:
- Line 8: Replace the mutable image tag under the YAML key "tag" (currently set
to "latest") with a pinned, immutable image identifier (e.g., a specific semver
tag or image digest) and update automation to substitute that fixed value during
e2e runs; ensure the change targets the "tag" field in this values.yaml so CI
uses a deterministic image version rather than "latest".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
testdata/adapter-configs/clusters-cl-maestro/adapter-config.yamltestdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yamltestdata/adapter-configs/clusters-cl-maestro/values.yaml
| #TODO: this should come from config.adapter.metadata.name | ||
| - name: "adapterName" | ||
| source: "config.adapterConfig.metadata.name" | ||
| default: "cl-maestro" |
There was a problem hiding this comment.
Resolve the TODO for adapterName source before merge.
Line 27 indicates the source path is still undecided. Please finalize the canonical source path and remove the TODO to avoid ambiguous config behavior.
If helpful, I can draft the exact patch once you confirm the intended source (config.adapter.metadata.name vs config.adapterConfig.metadata.name).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml` around
lines 27 - 30, The TODO indicates the source for the adapterName is ambiguous;
update the adapter task config so the "name: \"adapterName\"" entry uses the
canonical path "source: \"config.adapter.metadata.name\"" (replace the current
config.adapterConfig path), remove the TODO comment, and keep or adjust the
default "cl-maestro" as fallback; ensure the change is applied to the
adapterName block so any code reading adapterName will pull from
config.adapter.metadata.name.
| - name: "placementClusterName" | ||
| expression: "\"cluster1\"" # TBC coming from placement adapter | ||
| description: "Unique identifier for the target maestro" |
There was a problem hiding this comment.
Remove the hardcoded placement cluster.
Line 56 always sets placementClusterName to "cluster1", so Line 77 sends work to a fixed cluster regardless of event/context.
Also applies to: 77-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml` around
lines 55 - 57, The placementClusterName field is hardcoded via the expression
"\"cluster1\"" so work is always routed to a fixed cluster; change the
expression for placementClusterName to reference the runtime/context variable
(e.g., the event or placement value provided by the adapter) instead of a
literal string and update the code that reads placementClusterName (the logic
that sends work at the place referenced in the diff around line 77) to consume
that dynamic field so the target maestro is selected from the event/context
rather than "cluster1".
There was a problem hiding this comment.
hardcode it for now. For further E2E implementation, we need further PRs to make it happen. This is just a almost ready config to avoid QE automation from scratch.
There was a problem hiding this comment.
@xueli181114, understood — keeping the hardcoded value for now to get the E2E setup working makes sense. Would you like me to open a GitHub issue to track implementing dynamic placement cluster selection in a future PR? That way the follow-up work is documented and won't be forgotten.
|
The values.yaml structure doesn't match the other adapters or the deploy script expectations. For example, --set "image.registry=..." would set a top-level image.registry key, but the Compare with the existing adapters (cl-deployment, cl-job, cl-namespace) which all use a flat |
| # ManifestWork is a kind of manifest that can be used to create resources on the cluster. | ||
| # It is a collection of resources that are created together. | ||
| # It is created by the adapter and can be used to create resources on the cluster. | ||
| # It is created by the adapter and can be used to create resources on the cluster. |
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml (2)
27-30:⚠️ Potential issue | 🟡 MinorFinalize
adapterNamesource and remove the TODO.Line 27 still carries a TODO, and Line 29 still points to
config.adapterConfig.metadata.name. If the canonical path isconfig.adapter.metadata.name, this will keep pulling from the wrong config branch.Suggested patch
- `#TODO`: this should come from config.adapter.metadata.name - name: "adapterName" - source: "config.adapterConfig.metadata.name" + source: "config.adapter.metadata.name" default: "cl-maestro"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml` around lines 27 - 30, Update the adapterName entry so it reads from the canonical config path and remove the TODO: change the source value from "config.adapterConfig.metadata.name" to "config.adapter.metadata.name" for the name: "adapterName" entry, and delete the preceding "#TODO" comment so the YAML reflects the final source.
55-57:⚠️ Potential issue | 🟠 Major
placementClusterNameis still hardcoded, so routing stays static.Line 56 uses a literal
"cluster1", so Line 77 will always target the same cluster and ignore runtime placement context.Suggested direction
- - name: "placementClusterName" - expression: "\"cluster1\"" # TBC coming from placement adapter + - name: "placementClusterName" + expression: "event.placementClusterName" # use runtime placement fieldAlso applies to: 77-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml` around lines 55 - 57, The placementClusterName entry currently hardcodes the cluster ("\"cluster1\"") so runtime placement is ignored; change the expression for placementClusterName to reference the placement adapter/runtime placement variable (instead of the literal string) and update the corresponding usage that routes to the target maestro (the place that reads placementClusterName) to use that runtime value; specifically, replace the expression value for the placementClusterName field and the matching routing reference so they read from the placement context/input provided by the placement adapter rather than the hardcoded "\"cluster1\"" literal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yaml`:
- Around line 27-30: Update the adapterName entry so it reads from the canonical
config path and remove the TODO: change the source value from
"config.adapterConfig.metadata.name" to "config.adapter.metadata.name" for the
name: "adapterName" entry, and delete the preceding "#TODO" comment so the YAML
reflects the final source.
- Around line 55-57: The placementClusterName entry currently hardcodes the
cluster ("\"cluster1\"") so runtime placement is ignored; change the expression
for placementClusterName to reference the placement adapter/runtime placement
variable (instead of the literal string) and update the corresponding usage that
routes to the target maestro (the place that reads placementClusterName) to use
that runtime value; specifically, replace the expression value for the
placementClusterName field and the matching routing reference so they read from
the placement context/input provided by the placement adapter rather than the
hardcoded "\"cluster1\"" literal.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
testdata/adapter-configs/clusters-cl-maestro/adapter-task-config.yamltestdata/adapter-configs/clusters-cl-maestro/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- testdata/adapter-configs/clusters-cl-maestro/values.yaml
Oh, it's in different values format from infra. Updated. |
|
|
||
| # Documentation | ||
| description: "Complete cluster setup including namespace, configuration, and RBAC" | ||
| documentation: "https://docs.hyperfleet.io/adapters/aro-hcp" |
There was a problem hiding this comment.
Removed the confusion lines
xueli181114
force-pushed
the
HYPERFLEET-572
branch
from
87d63d2 to
25c5e83
Compare
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
testdata/adapter-configs/cl-maestro/adapter-config.yaml (1)
40-44:⚠️ Potential issue | 🟠 MajorUse adapter-specific Maestro IDs to prevent cross-adapter collisions.
Line 40 and Line 44 still use generic identifiers, despite the uniqueness requirement noted at Line 38.
Proposed patch
- sourceId: "hyperfleet-adapter" + sourceId: "cl-maestro" @@ - clientId: "hyperfleet-adapter-client" + clientId: "cl-maestro-client"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/cl-maestro/adapter-config.yaml` around lines 40 - 44, The config uses generic Maestro identifiers "sourceId" and "clientId" which can collide across adapters; update the values in adapter-config.yaml by replacing sourceId: "hyperfleet-adapter" and clientId: "hyperfleet-adapter-client" with adapter-unique Maestro IDs (e.g., include the adapter name/namespace or a GUID) so they are distinct per adapter; ensure both the sourceId and clientId fields are changed consistently to the new adapter-specific values used by your Maestro integration.testdata/adapter-configs/cl-maestro/adapter-task-config.yaml (2)
27-30:⚠️ Potential issue | 🟡 MinorResolve
adapterNamesource TODO before merge.Line 27 still has a TODO, and Line 29 uses
config.adapterConfig.metadata.nameinstead of the canonical adapter metadata source.Proposed patch
- `#TODO`: this should come from config.adapter.metadata.name - name: "adapterName" - source: "config.adapterConfig.metadata.name" + source: "config.adapter.metadata.name" default: "cl-maestro"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/cl-maestro/adapter-task-config.yaml` around lines 27 - 30, Replace the temporary source used for the adapterName entry and remove the TODO: update the "source" value for the adapterName field from "config.adapterConfig.metadata.name" to the canonical "config.adapter.metadata.name" and delete the comment/TODO line; ensure the "default" remains "cl-maestro" and that the key name "adapterName" is unchanged so any consumers of adapterName read from config.adapter.metadata.name going forward.
55-57:⚠️ Potential issue | 🟠 MajorRemove hardcoded placement cluster from routing path.
Line 56 hardcodes
placementClusterNameto"cluster1", so Line 77 always routes to a fixed target cluster.#!/bin/bash set -euo pipefail echo "Placement and targetCluster usage across adapter task configs:" rg -n --glob 'testdata/adapter-configs/**/adapter-task-config.yaml' 'placementClusterName|targetCluster|cluster1' echo echo "Current cl-maestro snippet:" rg -n -C2 'placementClusterName|targetCluster' testdata/adapter-configs/cl-maestro/adapter-task-config.yamlExpected result: no literal
"cluster1"in the placement expression for cl-maestro.Also applies to: 77-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/cl-maestro/adapter-task-config.yaml` around lines 55 - 57, The placementClusterName expression is hardcoded to "\"cluster1\"" which forces routing to a fixed target; update the placementClusterName value in adapter-task-config.yaml for cl-maestro to derive dynamically (e.g., reference the incoming variable/parameter used by your routing logic rather than a literal) so the routing path that uses placementClusterName/targetCluster reads the runtime value; replace the literal expression in the placementClusterName entry and ensure any references on the routing/targetCluster side consume that variable (and then run the ripgrep check to confirm no literal "cluster1" remains).
🧹 Nitpick comments (1)
testdata/adapter-configs/cl-maestro/values.yaml (1)
25-25: Avoidlatestfor reproducible e2e runs.Line 25 should be pinned to an immutable tested tag (or digest) to prevent drift across test runs.
Proposed patch
- tag: latest + tag: "CHANGE_ME" # set to immutable tested tag or image digest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/cl-maestro/values.yaml` at line 25, The helm values entry "tag: latest" in the cl-maestro values.yaml should be replaced with a concrete, immutable image reference to ensure reproducible e2e runs; update the tag field (the "tag" key in values.yaml for cl-maestro) to a tested semver tag (e.g., vX.Y.Z) or an image digest (sha256:...) and ensure the CI/test matrix uses that pinned value instead of "latest".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@testdata/adapter-configs/cl-maestro/adapter-task-config.yaml`:
- Around line 130-131: The documentation URL for the cl-maestro task config is
incorrect (it points to /adapters/aro-hcp); update the documentation field in
adapter-task-config.yaml for the cl-maestro task to the correct cl-maestro
documentation path (e.g., the cl-maestro docs URL) or remove the documentation
key until the proper doc exists so the documentation value matches the adapter
(look for the "documentation:" key adjacent to the "description:" entry).
In `@testdata/adapter-configs/cl-maestro/values.yaml`:
- Line 4: Update the file mapping entries in values.yaml so they point to the
actual relative files instead of the non-existent clusters-cl-maestro directory:
change the mapping key/value pairs that reference
"clusters-cl-maestro/adapter-config.yaml" and
"clusters-cl-maestro/adapter-task-config.yaml" to either "adapter-config.yaml"
and "adapter-task-config.yaml" or "./adapter-config.yaml" and
"./adapter-task-config.yaml" (whichever project convention is used) so that the
adapter-config.yaml and task-config.yaml keys resolve to
testdata/adapter-configs/cl-maestro/adapter-config.yaml and
adapter-task-config.yaml respectively.
---
Duplicate comments:
In `@testdata/adapter-configs/cl-maestro/adapter-config.yaml`:
- Around line 40-44: The config uses generic Maestro identifiers "sourceId" and
"clientId" which can collide across adapters; update the values in
adapter-config.yaml by replacing sourceId: "hyperfleet-adapter" and clientId:
"hyperfleet-adapter-client" with adapter-unique Maestro IDs (e.g., include the
adapter name/namespace or a GUID) so they are distinct per adapter; ensure both
the sourceId and clientId fields are changed consistently to the new
adapter-specific values used by your Maestro integration.
In `@testdata/adapter-configs/cl-maestro/adapter-task-config.yaml`:
- Around line 27-30: Replace the temporary source used for the adapterName entry
and remove the TODO: update the "source" value for the adapterName field from
"config.adapterConfig.metadata.name" to the canonical
"config.adapter.metadata.name" and delete the comment/TODO line; ensure the
"default" remains "cl-maestro" and that the key name "adapterName" is unchanged
so any consumers of adapterName read from config.adapter.metadata.name going
forward.
- Around line 55-57: The placementClusterName expression is hardcoded to
"\"cluster1\"" which forces routing to a fixed target; update the
placementClusterName value in adapter-task-config.yaml for cl-maestro to derive
dynamically (e.g., reference the incoming variable/parameter used by your
routing logic rather than a literal) so the routing path that uses
placementClusterName/targetCluster reads the runtime value; replace the literal
expression in the placementClusterName entry and ensure any references on the
routing/targetCluster side consume that variable (and then run the ripgrep check
to confirm no literal "cluster1" remains).
---
Nitpick comments:
In `@testdata/adapter-configs/cl-maestro/values.yaml`:
- Line 25: The helm values entry "tag: latest" in the cl-maestro values.yaml
should be replaced with a concrete, immutable image reference to ensure
reproducible e2e runs; update the tag field (the "tag" key in values.yaml for
cl-maestro) to a tested semver tag (e.g., vX.Y.Z) or an image digest
(sha256:...) and ensure the CI/test matrix uses that pinned value instead of
"latest".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
testdata/adapter-configs/cl-maestro/adapter-config.yamltestdata/adapter-configs/cl-maestro/adapter-task-config.yamltestdata/adapter-configs/cl-maestro/values.yaml
| adapterConfig: | ||
| create: true | ||
| files: | ||
| adapter-config.yaml: clusters-cl-maestro/adapter-config.yaml |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Mappings from values.yaml ==="
rg -n 'adapter-config.yaml:|task-config.yaml:' testdata/adapter-configs/cl-maestro/values.yaml
echo
echo "=== Actual adapter config files under testdata/adapter-configs ==="
fd -t f '\.(yaml|yml)$' testdata/adapter-configs | sort
echo
echo "=== Directory structure of testdata/adapter-configs ==="
find testdata/adapter-configs -type d | sortRepository: openshift-hyperfleet/hyperfleet-e2e
Length of output: 2055
Fix adapter file mappings to use correct relative paths.
Lines 4 and 11 reference clusters-cl-maestro/... paths, but the actual config files are in cl-maestro/... (same directory as values.yaml). The mapped paths should be either adapter-config.yaml and adapter-task-config.yaml, or ./adapter-config.yaml and ./adapter-task-config.yaml.
Verification
values.yaml mappings:
Line 4: adapter-config.yaml: clusters-cl-maestro/adapter-config.yaml
Line 11: task-config.yaml: clusters-cl-maestro/adapter-task-config.yaml
Actual files:
✓ testdata/adapter-configs/cl-maestro/adapter-config.yaml
✓ testdata/adapter-configs/cl-maestro/adapter-task-config.yaml
Missing:
✗ testdata/adapter-configs/clusters-cl-maestro/ (directory does not exist)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/adapter-configs/cl-maestro/values.yaml` at line 4, Update the file
mapping entries in values.yaml so they point to the actual relative files
instead of the non-existent clusters-cl-maestro directory: change the mapping
key/value pairs that reference "clusters-cl-maestro/adapter-config.yaml" and
"clusters-cl-maestro/adapter-task-config.yaml" to either "adapter-config.yaml"
and "adapter-task-config.yaml" or "./adapter-config.yaml" and
"./adapter-task-config.yaml" (whichever project convention is used) so that the
adapter-config.yaml and task-config.yaml keys resolve to
testdata/adapter-configs/cl-maestro/adapter-config.yaml and
adapter-task-config.yaml respectively.
There was a problem hiding this comment.
@xueli181114 Thank you! Let me know once you've updated the file paths in values.yaml so the mappings point to the correct locations (adapter-config.yaml and adapter-task-config.yaml instead of clusters-cl-maestro/...).
|
|
||
| clients: | ||
| hyperfleetApi: | ||
| baseUrl: http://hyperfleet-api:8000 |
There was a problem hiding this comment.
CHANGE_ME might be better.
| broker: | ||
| # These values are overridden at deploy time via env vars from Helm values | ||
| subscriptionId: "placeholder" | ||
| topic: "placeholder" |
There was a problem hiding this comment.
CHANGE_ME might be consistent with other adapter configurations.
| kind: ConfigMap | ||
| data: | ||
| cluster_id: "{{ .clusterId }}" | ||
| cluster_name: "{{ .clusterId }}" |
There was a problem hiding this comment.
I think cluster_name should be another value.
| adapterConfig: | ||
| create: true | ||
| files: | ||
| adapter-config.yaml: clusters-cl-maestro/adapter-config.yaml |
There was a problem hiding this comment.
Update clusters-cl-maestro to cl-maestro
| adapterTaskConfig: | ||
| create: true | ||
| files: | ||
| task-config.yaml: clusters-cl-maestro/adapter-task-config.yaml |
There was a problem hiding this comment.
Update clusters-cl-maestro to cl-maestro
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
testdata/adapter-configs/cl-maestro/values.yaml (1)
21-25: Prefer a non-mutable image tag for deterministic runs.Line 25 uses
latest, which can cause drift across CI runs. Consider forcing explicit version input.Suggested change
image: registry: CHANGE_ME repository: hyperfleet-adapter pullPolicy: Always - tag: latest + tag: CHANGE_ME🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testdata/adapter-configs/cl-maestro/values.yaml` around lines 21 - 25, The image tag is set to the mutable value "latest" (image.tag) which causes non-deterministic CI runs; change image.tag to require an explicit, immutable version (for example a semver string provided via Helm value or CI variable) and update any deployment/Helm invocation to pass that version rather than relying on "latest" so builds and deployments are reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@testdata/adapter-configs/cl-maestro/values.yaml`:
- Around line 21-25: The image tag is set to the mutable value "latest"
(image.tag) which causes non-deterministic CI runs; change image.tag to require
an explicit, immutable version (for example a semver string provided via Helm
value or CI variable) and update any deployment/Helm invocation to pass that
version rather than relying on "latest" so builds and deployments are
reproducible.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
testdata/adapter-configs/cl-maestro/adapter-config.yamltestdata/adapter-configs/cl-maestro/adapter-task-config.yamltestdata/adapter-configs/cl-maestro/values.yaml
✅ Files skipped from review due to trivial changes (1)
- testdata/adapter-configs/cl-maestro/adapter-config.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- testdata/adapter-configs/cl-maestro/adapter-task-config.yaml
|
@xueli181114: This pull request references HYPERFLEET-572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 86254860 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit c5bdb38
into
openshift-hyperfleet:main
5 participants
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit