[ci]: Add pure database config adapter#722
Conversation
Extract namespace database configuration behind an internal provider seam. The normal build uses swsscommon, while the pure build parses database_config.json and rejects unsupported global configurations explicitly. Run canonical packages with the pure build tag, prove CGO-free builds, reject SONiC-only dependency leaks, and remove the sibling checkout from hosted pure tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Validate test dependencies and build-tag placement, run a CGO-free build before hosted race tests, and preserve default namespace lookup behavior. Share the database configuration contract with the real swsscommon provider and publish its integration result separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new internal/dbconfig adapter layer to decouple SONiC/CGO-dependent database configuration (via swsscommon) from reusable namespace/derived logic, enabling “pure” (CGO-free) builds and tests that run outside the SONiC build environment.
Changes:
- Added
internal/dbconfigwith a provider contract and two implementations:!pure(swsscommon-backed) andpure(file-backeddatabase_config.jsonparsing). - Updated
sonic_db_configto preserve legacy APIs by delegating namespace-based behavior tointernal/dbconfig. - Strengthened pure CI validation (dependency leakage + build-tag placement checks) and adjusted Azure pipeline checkout/pathing.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sonic_db_config/db_config.go | Delegates namespace-based dbconfig APIs to internal/dbconfig while preserving legacy surface. |
| pure.mk | Adds pure-tag flags, dependency leakage checks, and build-tag placement checks to pure CI targets. |
| Makefile | Adds integration test/coverage runs for internal/dbconfig and separates dbconfig contract tests for JUnit. |
| internal/dbconfig/testdata/database_config.json | Adds standalone JSON config fixture used by the pure provider tests. |
| internal/dbconfig/provider_swss.go | Implements !pure provider backed by swsscommon. |
| internal/dbconfig/provider_swss_test.go | Adds provider contract test coverage for the swsscommon provider. |
| internal/dbconfig/provider_pure.go | Implements pure provider that reads/parses database_config.json. |
| internal/dbconfig/provider_pure_test.go | Adds unit tests validating pure-provider behavior and error paths. |
| internal/dbconfig/provider_contract_test.go | Adds shared provider contract test helper used by both providers. |
| internal/dbconfig/dbconfig.go | Introduces provider interface + common derived logic with initialization/reset control. |
| azure-pipelines.yml | Removes extra checkout for pure tests, updates paths, and publishes integration dbconfig JUnit output. |
Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
internal/dbconfig/provider_swss.go:19
- swssProvider.initialize() treats any os.Stat() error as equivalent to "file missing" (via os.IsExist) and falls back to SonicDBConfigInitialize(). If the global config file is unreadable (e.g., permission/IO error), this silently initializes the wrong config instead of surfacing the error. Use os.IsNotExist(err) to distinguish missing vs other errors and return the unexpected error.
func (swssProvider) initialize() error {
if _, err := os.Stat(GlobalConfigFile); err == nil || os.IsExist(err) {
if !swsscommon.SonicDBConfigIsGlobalInit() {
swsscommon.SonicDBConfigInitializeGlobalConfig()
}
Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
internal/dbconfig/provider_swss.go:16
os.IsExist(err)is not meaningful for errors returned byos.Statand can cause non-"not exist" errors (e.g., permission/IO) to be treated as “no global config”, silently falling back toSonicDBConfigInitialize(). Handleos.IsNotExist(err)explicitly and return unexpected errors.
if _, err := os.Stat(GlobalConfigFile); err == nil || os.IsExist(err) {
internal/dbconfig/provider_pure.go:62
- When
INCLUDESis present indatabase_config.json, the error message says “global database configuration”, but this case is specifically about unsupported includes. This makes failures harder to understand.
if len(p.config.Includes) > 0 {
return fmt.Errorf("global database configuration is not supported by the pure provider")
}
internal/dbconfig/provider_pure_test.go:41
%qis not a valid/meaningful format for a[]stringand will print%!q([]string=...). Use%v(or%#v) for slices in test failure messages.
t.Errorf("GetDbAllNamespaces() = %q, want default namespace", namespaces)
internal/dbconfig/provider_pure_test.go:49
%qis not a valid/meaningful format for a[]stringand will print%!q([]string=...). Use%v(or%#v) for slices in test failure messages.
t.Errorf("GetDbNonDefaultNamespaces() = %q, want none", nonDefault)
internal/dbconfig/provider_pure_test.go:66
%qis not a valid/meaningful format for a[]stringand will print%!q([]string=...). Use%v(or%#v) for slices in test failure messages.
t.Errorf("GetDbList() = %q, want %q", databases, wantDatabases)
internal/dbconfig/provider_pure_test.go:157
%qis not a valid/meaningful format for a[]stringand will print%!q([]string=...). Use%v(or%#v) for slices in test failure messages.
t.Errorf("GetDbAllNamespaces() = %q, want namespace %q", namespaces, contract.namespace)
internal/dbconfig/provider_pure_test.go:165
%qis not a valid/meaningful format for a[]stringand will print%!q([]string=...). Use%v(or%#v) for slices in test failure messages.
t.Errorf("GetDbList() = %q, want database %q", databases, contract.database)
Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Why I did it
This is the first adapter slice after
sonic-net/sonic-gnmi#719.
Canonical packages need a repeatable way to isolate dependencies that cannot
compile outside the SONiC build environment. Database configuration is the
first slice because its namespace API contains reusable derived logic while its
implementation depends on swsscommon.
How I did it
internal/dbconfig!pureprovider that wraps the existing swsscommon callspureprovider that parsesdatabase_config.jsonsonic_db_confignamespace signatures through delegateschecks to the pure workflow
How to verify it
make -f pure.mk cimake -f pure.mk junit-xmlmake -f pure.mk PACKAGES=internal/dbconfig check-deps check-tags build-test testmake check_gotest_junitin the SONiC integration environmentsonic-net/sonic-buildimage#28409
Which release branch to backport (provide reason below if selected)
Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): N/A
Failure type: N/A
Tested branch
Test result
#28409.
git range-diffreports both step-1 patches unchanged by the rebase.Description for the changelog
Add the first pure/real adapter seam for standalone database configuration
tests.
Link to config_db schema for YANG module changes
Not applicable.
A picture of a cute animal (not mandatory but encouraged)