fix noble acceptance tests#155
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThe PR makes acceptance tests and CI use ubuntu-jammy by default, adds helpers to detect base stemcell and map DNS behavior, introduces a jammy-specific ops manifest that overrides DNS settings, conditionally appends that ops file to BOSH deploy arguments in test deployment helpers and scripts, initializes a package-level baseStemcell during test setup, and skips or guards resolv.conf tests on non-jammy stemcells. Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 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 `@src/bosh-dns/acceptance_tests/linux/linux_test.go`:
- Around line 135-137: The AfterEach unconditionally moves
/tmp/resolv.conf.backup back which fails when BeforeEach skipped creating that
backup for non-ubuntu-jammy stemcells; update the AfterEach to guard restoration
with the same condition (check baseStemcell == "ubuntu-jammy") and/or verify the
backup exists before attempting mv (e.g., test for /tmp/resolv.conf.backup), so
the restore only runs when the BeforeEach created the backup; adjust the
AfterEach closure in linux_test.go accordingly around the restore logic.
🪄 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: af5bca34-107d-42f0-9abc-d3055e34b127
📒 Files selected for processing (7)
scripts/run-acceptance-testssrc/bosh-dns/acceptance_tests/init_test.gosrc/bosh-dns/acceptance_tests/linux/init_test.gosrc/bosh-dns/acceptance_tests/linux/linux_test.gosrc/bosh-dns/acceptance_tests/support_test.gosrc/bosh-dns/test_yml_assets/manifests/dns-linux.ymlsrc/bosh-dns/test_yml_assets/ops/manifest/jammy-dns-settings.yml
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
from
4f21150 to
eb08ec3
Compare
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
aramprice
reviewed
Jun 11, 2026
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
from
eb08ec3 to
1d43f3b
Compare
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)
src/bosh-dns/acceptance_tests/init_test.go (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix goimports formatting.
The CI pipeline reports that this file is not properly formatted.
Run the following to fix:
#!/bin/bash # Fix goimports formatting goimports -w src/bosh-dns/acceptance_tests/init_test.go🤖 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 `@src/bosh-dns/acceptance_tests/init_test.go` at line 22, The file has goimports formatting issues; run goimports to fix import organization and formatting for the file containing the symbol allDeployedInstances (declared as []helpers.InstanceInfo) — e.g., run `goimports -w src/bosh-dns/acceptance_tests/init_test.go` or apply equivalent formatting to reorder/remove unused imports and rewrite the file in place so CI passes.Source: Linters/SAST tools
🤖 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 `@scripts/run-acceptance-tests`:
- Around line 50-55: The OVERRIDE_NAMESERVER variable is unused; remove the
declaration and any assignments to OVERRIDE_NAMESERVER and keep only the
conditional that appends to DEPLOY_OPS when BASE_STEMCELL == "ubuntu-jammy"
(i.e., delete the OVERRIDE_NAMESERVER=false line and the
OVERRIDE_NAMESERVER=true assignment), leaving the DEPLOY_OPS modification (and
you may also remove the TODO comment about Jammy if you want to clean up the
block); reference the symbols OVERRIDE_NAMESERVER, BASE_STEMCELL, and DEPLOY_OPS
to locate the code to change.
In `@src/bosh-dns/acceptance_tests/init_test.go`:
- Around line 119-126: The function dnsSettingsForStemcell is duplicated;
refactor by extracting it into a single shared helper and updating both callers
to use that helper: create a new helper (e.g., package acceptancehelpers or
export the function from the parent acceptance package) that defines
dnsSettingsForStemcell(stemcell string) (overrideNameserver,
configureSystemdResolved bool) and replace the duplicated implementations in
init_test.go and linux/init_test.go to import and call the single shared
dnsSettingsForStemcell; ensure package visibility (export if needed) and update
imports accordingly.
In `@src/bosh-dns/acceptance_tests/linux/init_test.go`:
- Around line 25-30: The file's imports/formatting are out of goimports style
causing CI failure; run goimports to rewrite imports and format the file (or
manually fix import grouping/unused imports) so the declaration block containing
boshBinaryPath, allDeployedInstances, firstInstanceSlug, baseStemcell,
overrideNameserver, and configureSystemdResolved conforms to goimports
formatting; after fixing, verify with goimports -w or gofmt and re-run CI to
ensure the file is properly formatted.
---
Outside diff comments:
In `@src/bosh-dns/acceptance_tests/init_test.go`:
- Line 22: The file has goimports formatting issues; run goimports to fix import
organization and formatting for the file containing the symbol
allDeployedInstances (declared as []helpers.InstanceInfo) — e.g., run `goimports
-w src/bosh-dns/acceptance_tests/init_test.go` or apply equivalent formatting to
reorder/remove unused imports and rewrite the file in place so CI passes.
🪄 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: cfed3bd7-3796-4ccf-bfa0-59ba9df76691
📒 Files selected for processing (7)
scripts/run-acceptance-testssrc/bosh-dns/acceptance_tests/init_test.gosrc/bosh-dns/acceptance_tests/linux/init_test.gosrc/bosh-dns/acceptance_tests/linux/linux_test.gosrc/bosh-dns/acceptance_tests/support_test.gosrc/bosh-dns/test_yml_assets/manifests/dns-linux.ymlsrc/bosh-dns/test_yml_assets/ops/manifest/override-nameserver-dns-settings.yml
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
3 times, most recently
from
d2ce401 to
5d40b6a
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Pull request overview
This PR aims to make the test-acceptance-ubuntu-noble job green by defaulting the acceptance-test deployment manifest to the systemd-resolved–integrated bosh-dns configuration on Noble, while applying a Jammy-specific ops-file override and aligning acceptance tests/scripts with the stemcell-specific behavior.
Changes:
- Switch the Linux acceptance manifest defaults to
configure_systemd_resolved: trueandoverride_nameserver: false, and add a Jammy override ops file. - Update acceptance test deployment helpers to optionally include the stemcell-specific ops file; skip
/etc/resolv.confmutation tests when nameserver override is disabled. - Update
scripts/run-acceptance-teststo compose deploy ops files and apply the Jammy override conditionally.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/bosh-dns/test_yml_assets/ops/manifest/override-nameserver-dns-settings.yml | New ops file to revert bosh-dns settings for Jammy-style resolver behavior. |
| src/bosh-dns/test_yml_assets/manifests/dns-linux.yml | Changes default bosh-dns properties to systemd-resolved integration defaults (Noble-oriented). |
| src/bosh-dns/acceptance_tests/support_test.go | Conditionally applies the stemcell-specific ops file during deploys from acceptance tests. |
| src/bosh-dns/acceptance_tests/linux/linux_test.go | Skips /etc/resolv.conf mutation tests when override is disabled (systemd-resolved-managed). |
| src/bosh-dns/acceptance_tests/linux/init_test.go | Reads BASE_STEMCELL and derives DNS settings for conditional test behavior. |
| src/bosh-dns/acceptance_tests/init_test.go | Derives stemcell DNS settings and adds helper to select a stemcell ops file. |
| src/bosh-dns/acceptance_tests/helpers/settings.go | Adds stemcell-to-DNS-settings mapping helper used by acceptance tests. |
| scripts/run-acceptance-tests | Updates deploy ops composition and applies Jammy override conditionally. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
+13
| func DNSSettingsForStemcell(stemcell string) (overrideNameserver, configureSystemdResolved bool) { | ||
| switch stemcell { | ||
| case "ubuntu-jammy": // TODO: remove when Jammy goes EOL | ||
| return true, false | ||
| default: // ubuntu-noble and later | ||
| return false, true | ||
| } | ||
| } |
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
from
5d40b6a to
db64ec3
Compare
Comment on lines
10
to
12
| params: | ||
| BASE_STEMCELL: ubuntu-bionic | ||
| BASE_STEMCELL: ubuntu-jammy | ||
|
|
aramprice
reviewed
Jun 12, 2026
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
from
db64ec3 to
7f38ed7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/bosh-dns/acceptance_tests/support_test.go`:
- Around line 77-89: In ensureRecursorIsDefinedByBoshAgent,
ensureRecursorIsDefinedByDNSRelease, and ensureHealthEndpointDeployed the
manifestPath is placed before appended ops files, producing an invalid BOSH CLI
invocation; fix by removing manifestPath from the initial args slice and
appending it only after you append any ops files (including
stemcellDNSOpsFile()/assetPath(ops), any extraOps passed to
ensureHealthEndpointDeployed, and the "-o" flags), then call
helpers.Bosh(args...) so all flags precede the manifest path.
🪄 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: bbf03624-b661-44e0-91f1-f00a8ed6b174
📒 Files selected for processing (11)
ci/tasks/test-acceptance.ymlscripts/run-acceptance-testsscripts/test-acceptancesrc/bosh-dns/acceptance_tests/helpers/settings.gosrc/bosh-dns/acceptance_tests/init_test.gosrc/bosh-dns/acceptance_tests/linux/init_test.gosrc/bosh-dns/acceptance_tests/linux/linux_test.gosrc/bosh-dns/acceptance_tests/support_test.gosrc/bosh-dns/test_yml_assets/manifests/dns-linux.ymlsrc/bosh-dns/test_yml_assets/ops/manifest/override-nameserver-dns-settings.ymlsrc/bosh-dns/test_yml_assets/ops/manifest/use-dns-release-default-bind-and-alias-addresses.yml
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
The test-acceptance-ubuntu-noble job has never been green. This hopefully will fix it. Changing the default settings in the build to be a systemd integrated bosh-dns, and applying an ops file on Jammy which can be removed when Jammy goes out of support.
mkocher
force-pushed
the
fix-noble-acceptance-tests
branch
from
7f38ed7 to
1b21c8a
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
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
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.
The test-acceptance-ubuntu-noble job has never been green. This hopefully will fix it.
Changing the default settings in the build to be a systemd integrated bosh-dns, and applying an ops file on Jammy which can be removed when Jammy goes out of support.