Skip to content

feat: minimal mlab-ns compatibility shim#252

Merged
bassosimone merged 2 commits intomainfrom
sandbox-sbs-mlabns-emul
Mar 3, 2026
Merged

feat: minimal mlab-ns compatibility shim#252
bassosimone merged 2 commits intomainfrom
sandbox-sbs-mlabns-emul

Conversation

@bassosimone
Copy link
Copy Markdown
Collaborator

@bassosimone bassosimone commented Feb 25, 2026

We implement a minimal mlab-ns compatibility shim.

The service has been EOL for a long time now. We recently reached out to integrators and it seems there are still some that need a small compatibility shim to avoid breaking legacy applications.

Their migration is fully underway, but there needs to be time to migrate a fleet of legacy clients, which are using the output of mlab-ns for archival purposes.

Accordingly, this pull request implements just the minimum functionality to avoid breaking those applications. Yet, note that the returned response is not actionable anymore for running ndt5 tests (and it can't be used for running ndt7 tests either).

The complete plan is this:

  1. merge this diff

  2. deploy

  3. use dispatch.yaml to route to locate

  4. discontinue mlab-ns

  5. wait for the legacy applications queue to drain

  6. circle back and zap this diff

This is probably the last chapter in mlab-ns glorious life.

See #185.

Adapted from #184.


This change is Reviewable

@bassosimone
Copy link
Copy Markdown
Collaborator Author

I am now looking into the failures. Let's first deal with the linting failures:

  Error: handler/mlabnscompat.go:252:5: sloppyLen: len(v2Result.Results) <= 0 can be len(v2Result.Results) == 0 (gocritic)
  	if len(v2Result.Results) <= 0 || v2Result.Results[0].Location == nil {
  	   ^
  Error: handler/mlabnscompat_test.go:93:16: httpNoBody: http.NoBody should be preferred to the nil request body (gocritic)
  			outerReq := httptest.NewRequest(tt.method, tt.url, nil)
  			            ^
  Error: handler/mlabnscompat_test.go:364:16: httpNoBody: http.NoBody should be preferred to the nil request body (gocritic)
  			req, err := http.NewRequest(http.MethodGet, srv.URL+tt.path, nil)
  			            ^
  3 issues:
  * gocritic: 3

I agree with the second and the third. The first one, TBH, is a matter of code style and I am not super happy about this level of exaggerated linting for arguably more correct code (<= 0 excludes the whole domain of invalid values and len returns int, so, IMHO that's just a good coding practice across languages).

With respect to the tests proper, the failure is this:


/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOROOT
/opt/hostedtoolcache/go/1.25.7/x64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOPATH
/home/runner/go
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOBIN

/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOTMPDIR

/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOTOOLDIR
/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOOS
linux
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOARCH
amd64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOCACHE
/home/runner/.cache/go-build
/home/runner/work/_actions/shogo82148/actions-goveralls/v1/bin/goveralls_linux_amd64 -service=github -coverprofile=coverage.out
bad response status from coveralls: 530
error code: 1016
Error: Error: The process '/home/runner/work/_actions/shogo82148/actions-goveralls/v1/bin/goveralls_linux_amd64' failed with exit code 1

and it happens while uploading to coveralls. So, this also needs to be investigated.

@bassosimone bassosimone force-pushed the sandbox-sbs-mlabns-emul branch 2 times, most recently from 6fc738e to f0b4771 Compare February 25, 2026 13:06
@bassosimone
Copy link
Copy Markdown
Collaborator Author

bassosimone commented Feb 25, 2026

I am now looking into the failures. Let's first deal with the linting failures:

  Error: handler/mlabnscompat.go:252:5: sloppyLen: len(v2Result.Results) <= 0 can be len(v2Result.Results) == 0 (gocritic)
  	if len(v2Result.Results) <= 0 || v2Result.Results[0].Location == nil {
  	   ^
  Error: handler/mlabnscompat_test.go:93:16: httpNoBody: http.NoBody should be preferred to the nil request body (gocritic)
  			outerReq := httptest.NewRequest(tt.method, tt.url, nil)
  			            ^
  Error: handler/mlabnscompat_test.go:364:16: httpNoBody: http.NoBody should be preferred to the nil request body (gocritic)
  			req, err := http.NewRequest(http.MethodGet, srv.URL+tt.path, nil)
  			            ^
  3 issues:
  * gocritic: 3

I agree with the second and the third. The first one, TBH, is a matter of code style and I am not super happy about this level of exaggerated linting for arguably more correct code (<= 0 excludes the whole domain of invalid values and len returns int, so, IMHO that's just a good coding practice across languages).

Fixed the second and the third one. Disable sloppyLen. We're already using staticcheck, which catches the real useful cases len(x) >= 0 and len(x) < 0 already. The len(x) <= 0 is valid code and I actually not only like it more but have trouble not using this pattern (maybe C++ habit, IDK, but being defensive is good mental habit).

@bassosimone
Copy link
Copy Markdown
Collaborator Author

With respect to the tests proper, the failure is this:


/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOROOT
/opt/hostedtoolcache/go/1.25.7/x64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOPATH
/home/runner/go
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOBIN

/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOTMPDIR

/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOTOOLDIR
/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOOS
linux
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOARCH
amd64
/opt/hostedtoolcache/go/1.25.7/x64/bin/go env GOCACHE
/home/runner/.cache/go-build
/home/runner/work/_actions/shogo82148/actions-goveralls/v1/bin/goveralls_linux_amd64 -service=github -coverprofile=coverage.out
bad response status from coveralls: 530
error code: 1016
Error: Error: The process '/home/runner/work/_actions/shogo82148/actions-goveralls/v1/bin/goveralls_linux_amd64' failed with exit code 1

and it happens while uploading to coveralls. So, this also needs to be investigated.

This is a coveralls outage and there's not much that we can do but wait 🙄

@bassosimone
Copy link
Copy Markdown
Collaborator Author

@robertodauria coveralls is up and running again! Please, take a look! 🙏

@bassosimone bassosimone force-pushed the sandbox-sbs-mlabns-emul branch from 88b456d to f0b4771 Compare March 2, 2026 10:16
We implement a minimal mlab-ns compatibility shim.

The service has been EOL for a long time now. We recently
reached out to integrators and it seems there are still some
that need a small compatibility shim to avoid breaking
legacy applications.

Their migration is fully underway, but there needs to be time
to migrate a fleet of legacy clients, which are using the output
of mlab-ns for archival purposes.

Accordingly, this pull request implements just the minimum
functionality to avoid breaking those applications. Yet, note
that the returned response is not actionable anymore for
running ndt5 tests (and it can't be used for running ndt7
tests either).

The complete plan is this:

1. merge this diff

2. deploy

3. use `dispatch.yaml` to route to `locate`

4. discontinue `mlab-ns`

5. wait for the legacy applications queue to drain

6. circle back and zap this diff

This is probably the last chapter in mlab-ns glorious life.

See #185.

Adapted from #184.
@bassosimone bassosimone force-pushed the sandbox-sbs-mlabns-emul branch from f0b4771 to ba23c75 Compare March 2, 2026 10:18
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Mar 2, 2026

Pull Request Test Coverage Report for Build 22629741708

Details

  • 114 of 123 (92.68%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 82.629%

Changes Missing Coverage Covered Lines Changed/Added Lines %
locate.go 0 9 0.0%
Totals Coverage Status
Change from base Build 22571383483: 0.4%
Covered Lines: 2502
Relevant Lines: 3028

💛 - Coveralls

Copy link
Copy Markdown
Contributor

@robertodauria robertodauria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit.

@bassosimone bassosimone merged commit d464c64 into main Mar 3, 2026
9 checks passed
@bassosimone bassosimone deleted the sandbox-sbs-mlabns-emul branch March 3, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants