add integration for noble and resolute stemcells#455
Conversation
ai-assisted=yes [TNZ-88995] [Europa] Bosh KTLO
|
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:
WalkthroughThe CI pipeline adds separate Azure integration resources and jobs for Ubuntu resolute and noble, while switching the existing Azure integration job to the jammy deployment. Cleanup now targets variant-specific CredHub entries, resurrection configurations, and deployments. Integration service control supports systemd or Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/pipeline.yml`:
- Line 332: Update the tags value in the pipeline YAML to comply with the
configured bracket-spacing rule by removing the spaces inside the flow-style
list brackets, while preserving the existing broadcom tag.
- Around line 324-325: Update the test-integration-resolute and test-integration
job definitions to share the same serial_groups value for their
integration-vm-deployment resource, ensuring Azure integration deployments run
one at a time; retain each job’s existing serial setting and configuration.
- Around line 364-369: Update the stemcell_os value in the deployment
configuration alongside the azure-stemcell-resolute artifact to use the Resolute
OS name, ensuring it matches the uploaded stemcell and preserves the existing
deployment_name and az settings.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8d7686e4-c8eb-475d-9d18-cde6f74efca2
📒 Files selected for processing (1)
ci/pipeline.yml
| - name: test-integration-resolute | ||
| serial: true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
fly validate-pipeline --config ci/pipeline.yml
sed -n '320,570p' ci/pipeline.ymlRepository: cloudfoundry/bosh-agent
Length of output: 202
🏁 Script executed:
#!/bin/sh
set -eu
echo '--- ci/pipeline.yml: around the two integration jobs ---'
sed -n '300,590p' ci/pipeline.yml
echo
echo '--- serial / serial_groups usage in ci/pipeline.yml ---'
rg -n 'serial_groups|serial:' ci/pipeline.yml
echo
echo '--- references to bosh-agent-integration-main and integration-vm-deployment ---'
rg -n 'bosh-agent-integration-main|integration-vm-deployment|test-integration-resolute|test-integration' ci/pipeline.ymlRepository: cloudfoundry/bosh-agent
Length of output: 16420
Serialize the Azure integration jobs together.
serial: true only serializes builds within each job. test-integration-resolute and test-integration both deploy to and delete bosh-agent-integration-main through integration-vm-deployment, so they can still run at the same time and tear down each other’s environment. Add the same serial_groups value to both jobs, or isolate Resolute’s deployment/resource.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci/pipeline.yml` around lines 324 - 325, Update the test-integration-resolute
and test-integration job definitions to share the same serial_groups value for
their integration-vm-deployment resource, ensuring Azure integration deployments
run one at a time; retain each job’s existing serial setting and configuration.
| trigger: true | ||
| - get: bosh-agent-ci | ||
| - get: bosh-ecosystem-concourse | ||
| tags: [ broadcom ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the YAMLlint failure.
[ broadcom ] violates the configured bracket-spacing rule.
Proposed fix
- tags: [ broadcom ]
+ tags: [broadcom]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tags: [ broadcom ] | |
| tags: [broadcom] |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 332-332: too many spaces inside brackets
(brackets)
[error] 332-332: too many spaces inside brackets
(brackets)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci/pipeline.yml` at line 332, Update the tags value in the pipeline YAML to
comply with the configured bracket-spacing rule by removing the spaces inside
the flow-style list brackets, while preserving the existing broadcom tag.
Source: Linters/SAST tools
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci/pipeline.yml (1)
324-336: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate
bosh-integration-testsontest-integration-resolute. Inci/pipeline.yml:716-739, addtest-integration-resoluteto thepassedlists for bothbosh-agentandbosh-agent-registry-image; otherwise those versions can advance even when the Resolute job fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipeline.yml` around lines 324 - 336, Update the `passed` lists for both `bosh-agent` and `bosh-agent-registry-image` in the `bosh-integration-tests` job to include `test-integration-resolute`, ensuring those resources cannot advance until the Resolute integration job succeeds.
♻️ Duplicate comments (1)
ci/pipeline.yml (1)
324-325: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSerialize the Azure integration jobs together.
serial: trueonly queues builds within one job. Sincetest-integration-resoluteandtest-integrationboth modify and deletebosh-agent-integration-mainthroughintegration-vm-deployment, they can still overlap and tear down each other’s environment. Add the sameserial_groupsvalue to both jobs, or isolate their deployment/resource. (concourse-ci.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipeline.yml` around lines 324 - 325, Update the test-integration-resolute and test-integration job definitions to share the same serial_groups value, ensuring their integration-vm-deployment operations cannot overlap; retain each job’s existing serial setting and use a consistent group name for both.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ci/pipeline.yml`:
- Around line 324-336: Update the `passed` lists for both `bosh-agent` and
`bosh-agent-registry-image` in the `bosh-integration-tests` job to include
`test-integration-resolute`, ensuring those resources cannot advance until the
Resolute integration job succeeds.
---
Duplicate comments:
In `@ci/pipeline.yml`:
- Around line 324-325: Update the test-integration-resolute and test-integration
job definitions to share the same serial_groups value, ensuring their
integration-vm-deployment operations cannot overlap; retain each job’s existing
serial setting and use a consistent group name for both.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8c41e904-71d4-4cc9-96bb-e3485bea5e47
📒 Files selected for processing (1)
ci/pipeline.yml
KauzClay
changed the title
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
resolute stemcells use systemctl instead of runit for managing agent process ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
* the tooling already existed to change the name, it was just using bosh-agent-integration-main hardcoded everywhere since there was only one job ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ci/pipeline.yml (1)
353-364: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate releases on the new stemcell jobs.
Adding these jobs alone does not make them release-blocking:
bosh-integration-testsstill lists only Jammy and Windows jobs in bothbosh-agentandbosh-agent-registry-imagepassedconstraints. A Noble or Resolute failure can therefore be bypassed by promotion. Addtest-integration-resoluteandtest-integration-nobleto both lists.Proposed fix
passed: - test-unit + - test-integration-resolute + - test-integration-noble - test-integration - test-unit-windows - test-integration-windowsAlso applies to: 484-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipeline.yml` around lines 353 - 364, Update the bosh-integration-tests passed constraints for both bosh-agent and bosh-agent-registry-image to include test-integration-resolute and test-integration-noble alongside the existing Jammy and Windows jobs, making Noble or Resolute failures block promotion.integration/test_environment.go (1)
665-675: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMove diagnostics before the successful return.
The
return nilat Line 667 bypasses the new log and journal output whenever the agent becomes reachable. The diagnostics currently run only after all ping attempts fail, so the stated successful-startup diagnostic behavior is not implemented.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration/test_environment.go` around lines 665 - 675, Update the agent startup retry flow around t.AgentClient.Ping so the logs and journal diagnostics execute before returning success when the agent becomes reachable. Preserve the existing retry behavior, but move the return nil after the diagnostic output so successful startup also emits both t.GetFileContents and t.RunCommand results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration/test_environment.go`:
- Line 670: Run goimports on the changed section in
integration/test_environment.go and apply its formatting/import adjustments,
preserving the existing behavior and ensuring the file passes the goimports lint
check.
- Around line 671-675: Update the diagnostic collection around t.GetFileContents
and t.RunCommand to capture and handle their errors instead of discarding them.
Gate the journalctl diagnostic to systemctl environments, and ensure failures
from journalctl are not masked by the tail pipeline; preserve the existing log
and journal output formatting for successful commands.
- Around line 614-619: Update StartAgent to return the error produced by
t.RunCommand, matching the existing StopAgent behavior and satisfying the
method’s error return contract.
---
Outside diff comments:
In `@ci/pipeline.yml`:
- Around line 353-364: Update the bosh-integration-tests passed constraints for
both bosh-agent and bosh-agent-registry-image to include
test-integration-resolute and test-integration-noble alongside the existing
Jammy and Windows jobs, making Noble or Resolute failures block promotion.
In `@integration/test_environment.go`:
- Around line 665-675: Update the agent startup retry flow around
t.AgentClient.Ping so the logs and journal diagnostics execute before returning
success when the agent becomes reachable. Preserve the existing retry behavior,
but move the return nil after the diagnostic output so successful startup also
emits both t.GetFileContents and t.RunCommand results.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8ca8329a-c6d9-4aa3-bb30-b827a0c15978
📒 Files selected for processing (4)
ci/pipeline.ymlci/tasks/test-integration.shintegration/system_mounts_test.gointegration/test_environment.go
all the tests are failing bc the agent config the tests use doesn't set a serivce manager, so it tries using sv instead of systemd not sure the best way to thread this all through, since the agent settings file is used in lots of tests ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
including test pipeline, should delete before merge ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
builds the filename based on specification input and the service manager being used ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
|
|
||
| logs, _ := t.GetFileContents("/var/vcap/bosh/log/current") | ||
| t.writerPrinter.Printf("\n--- AGENT LOGS ---\n%s\n------------------\n", logs) | ||
|
|
||
| journal, _ := t.RunCommand("sudo journalctl -u agent --no-pager | tail -n 100") | ||
| t.writerPrinter.Printf("\n--- SYSTEMD JOURNAL ---\n%s\n-----------------------\n", journal) | ||
|
|
There was a problem hiding this comment.
TODO: these are debug changes, remove them before merging
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci/pipeline.yml (1)
353-364: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd
serial: trueto all three Azure integration jobs. Each job triggers onbosh-agent/stemcell resources andputs to a per-variant deployment with no job-level serialization, so concurrent builds of the same job clobber each other's environment. One fix pattern applies to all three.
ci/pipeline.yml#L353-L364: addserial: truetotest-integration-resolute(protectsintegration-vm-deployment-resolute).ci/pipeline.yml#L485-L496: addserial: truetotest-integration-noble(protectsintegration-vm-deployment-noble).ci/pipeline.yml#L617-L747: addserial: truetotest-integration(protectsintegration-vm-deployment-jammy).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/pipeline.yml` around lines 353 - 364, Add job-level serial execution to test-integration-resolute, test-integration-noble, and test-integration in ci/pipeline.yml at lines 353-364, 485-496, and 617-747 respectively, setting serial: true for each job to prevent concurrent builds from clobbering their variant-specific deployments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/test-delete-when-finished-resolute.yml`:
- Line 1: Remove the temporary ci/test-delete-when-finished-resolute.yml
pipeline file before merging, including its branch-specific resource pins; do
not modify the YAMLlint issues within it.
In `@integration/assets/file-settings-agent-no-default-tmp-dir-systemd.json`:
- Around line 2-8: Add Platform.Linux.ServiceManager with the value "systemd" to
the Linux settings object in the no-default-tmp-dir asset. Keep the existing
Linux settings unchanged so GetSettingsFile("no-default-tmp-dir") explicitly
exercises the systemd environment.
In `@integration/test_environment.go`:
- Around line 638-649: Update the shell command in
TestEnvironment.DetectServiceManager to terminate the else branch correctly with
a command separator before fi, while preserving the existing sv/systemd
detection and serviceManager assignment behavior.
---
Outside diff comments:
In `@ci/pipeline.yml`:
- Around line 353-364: Add job-level serial execution to
test-integration-resolute, test-integration-noble, and test-integration in
ci/pipeline.yml at lines 353-364, 485-496, and 617-747 respectively, setting
serial: true for each job to prevent concurrent builds from clobbering their
variant-specific deployments.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 1212e7c5-5ba1-4be7-b5d5-8d29adc43fe2
📒 Files selected for processing (25)
ci/pipeline.ymlci/test-delete-when-finished-resolute.ymlintegration/apply_test.gointegration/assets/file-settings-agent-no-default-tmp-dir-systemd.jsonintegration/assets/file-settings-agent-systemd.jsonintegration/bundle_logs_test.gointegration/compile_package_test.gointegration/delete_arp_entries_test.gointegration/ephemeral_disk_test.gointegration/fetch_logs_test.gointegration/fetch_logs_with_signed_url_test.gointegration/file_settings_test.gointegration/instance_info_test.gointegration/integration_suite_test.gointegration/nats_firewall_test.gointegration/prepare_test.gointegration/raw_ephemeral_disk_test.gointegration/remove_file_test.gointegration/run_script_test.gointegration/sync_dns_test.gointegration/system_mounts_test.gointegration/test_environment.gointegration/update_settings_test.gointegration/user_permissions_test.gointegration/v1_apply_test.go
| @@ -0,0 +1,508 @@ | |||
| # TODO: Delete this test pipeline file before merging. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Temporary pipeline file — must be removed before merge.
This file is explicitly marked for deletion and pins resources to the resolute-integ-test feature branch. It should not land on the target branch. This also makes the YAMLlint bracket-spacing errors at lines 34, 121, 253, and 385 moot.
Want me to open a tracking issue to ensure this file is removed before merge?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci/test-delete-when-finished-resolute.yml` at line 1, Remove the temporary
ci/test-delete-when-finished-resolute.yml pipeline file before merging,
including its branch-specific resource pins; do not modify the YAMLlint issues
within it.
Source: Linters/SAST tools
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
rsyslog was disabled in order to deal with issues unmounting the data dir however, resolute requires rsyslog to populate the bosh agent log where the tests expect this reworks things to only disable rsyslog when necessary (and turn it back on) when running on resolute ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
the configs were the same, so I used no-default-tmp-dir in both places since that works with our new servicemanager aware logic ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
…uldn't come up Bootstrap failures that occur before SetupLoggingAndAuditing runs (see agent/bootstrap.go) happen before rsyslog is restarted, so they never reach /var/vcap/bosh/log/current - they only ever land in the journal. Fall back to journalctl for that case, without weakening the primary file-based check that exercises the real logging pipeline. ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
despite a file existing called bosh-start-logging-and-auditing, that file just starts auditing. The more you know ai-assisted=yes [TNZ-94258]
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
instead of grepping directly with the differences between jammy and resolute, using the helper should be the most consistent way to detect these changes ai-assisted=yes [TNZ-106038] Refactor bosh-agent CI pipeline to run integration tests on all supported stemcell lines (Jammy, Noble, Resolute)
tests are green at this point (with the stemcell fix to make systemd forward to rsyslog). Cleaning up now
6bfce95 to
01f21fc
Compare
2 participants
What is this change about?
Adding an integration test specifically to use the agent on resolute stemcell (and soon the noble stemcell as well)
In the past, only testing one stemcell line in this pipeline hid some issues from us till they caused breakages further down the line. This is to help us catch issues earlier.
example of new job: https://ci.bosh-ecosystem.cf-app.com/teams/main/pipelines/bosh-agent-main/jobs/test-integration-resolute
What tests have you run against this PR?
iterating here: https://ci.bosh-ecosystem.cf-app.com/teams/main/pipelines/test-delete-when-finished-integration-resolute
How should this change be described in bosh-agent release notes?
N/A
Does this PR introduce a breaking change?
NO
Tag your pair, your PM, and/or team!
@mkocher
AI Review Feedback
All AI review comments (CodeRabbit, and Copilot when assigned) must be resolved before human reviewers will look at the PR. For each comment, either: