Skip to content

Remove feature flag "windows-dns-proxy"#48738

Merged
thaJeztah merged 1 commit intomoby:masterfrom
robmry:47732_remove_windows-dns-proxy_featflag
Oct 24, 2024
Merged

Remove feature flag "windows-dns-proxy"#48738
thaJeztah merged 1 commit intomoby:masterfrom
robmry:47732_remove_windows-dns-proxy_featflag

Conversation

@robmry
Copy link
Contributor

@robmry robmry commented Oct 23, 2024

- What I did

Added in 26.1.0, commit 6c68be2
Default changed to true in 27.0.0, commit 33f9a53

- How I did it

No sign of problems so, removed.

- How to verify it

It doesn't do anything now.

- Description for the changelog

- Removed feature flag `windows-dns-proxy`, which was introduced in release 26.1.0 to enable forwarding
  to external DNS resolvers from Windows containers, to make `nslookup` work. It was enabled by default
  in release 27.0.0.

@robmry robmry added area/networking Networking impact/changelog kind/refactor PR's that refactor, or clean-up code area/networking/dns Networking labels Oct 23, 2024
@robmry robmry added this to the 28.0.0 milestone Oct 23, 2024
@robmry robmry self-assigned this Oct 23, 2024
thaJeztah
thaJeztah reviewed Oct 23, 2024
View reviewed changes
daemon/config/config_linux.go
}

if _, ok := conf.Features["windows-dns-proxy"]; ok {
return errors.New("feature option 'windows-dns-proxy' is only available on Windows")
Copy link
Member

@thaJeztah thaJeztah Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure how likely it is, but the only thing we could do is to (in the Windows variant of this) either return an error, or to log a Error / Warning to surface that the user is using an option that's no longer taken into account; given that the default is "true", that should probably only be relevant if they have it configured and set to false;

moby/daemon/config/config.go

Lines 687 to 693 in 43bbacb

if config.CorsHeaders != "" {
// TODO(thaJeztah): option is used to produce error when used; remove in next release
return errors.New(`DEPRECATED: The "api-cors-header" config parameter and the dockerd "--api-cors-header" option have been removed; use a reverse proxy if you need CORS headers`)
}
// validate platform-specific settings
return config.ValidatePlatformConfig()

Alternative to logging, we could add it to the Warnings array on the /info endpoint, which shows up in the output of docker info;

moby/daemon/info.go

Lines 46 to 50 in 43bbacb

func (daemon *Daemon) SystemInfo(ctx context.Context) (*system.Info, error) {
defer metrics.StartTimer(hostInfoFunctions.WithValues("system_info"))()
sysInfo := daemon.RawSysInfo()
cfg := daemon.config()

moby/daemon/info.go

Lines 153 to 165 in 43bbacb

func (daemon *Daemon) fillDriverInfo(v *system.Info) {
v.Driver = daemon.imageService.StorageDriver()
v.DriverStatus = daemon.imageService.LayerStoreStatus()
const warnMsg = `
WARNING: The %s storage-driver is deprecated, and will be removed in a future release.
Refer to the documentation for more information: https://docs.docker.com/go/storage-driver/`
switch v.Driver {
case "overlay":
v.Warnings = append(v.Warnings, fmt.Sprintf(warnMsg, v.Driver))
}

Copy link
Contributor Author

@robmry robmry Oct 24, 2024

Choose a reason for hiding this comment

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

Ok, sure - I've added a docker info warning (and I'll mark this as ready for review!), how does this look? ...

PS C:\ProgramData\docker> docker info
Client:
 Version:    27.3.0-rc.1
[...]
 Insecure Registries:
  ::1/128
  127.0.0.0/8
 Live Restore Enabled: false


WARNING: Feature flag "windows-dns-proxy" has been removed, forwarding to external DNS resolvers is enabled.

@robmry
Remove feature flag "windows-dns-proxy"
b79bba6 
Added in 26.1.0, commit 6c68be2
Default changed to true in 27.0.0, commit 33f9a53

No sign of problems so, remove.

Signed-off-by: Rob Murray <[email protected]>
@robmry robmry force-pushed the 47732_remove_windows-dns-proxy_featflag branch from 4f65a0d to b79bba6 Compare October 24, 2024 10:20
@robmry robmry marked this pull request as ready for review October 24, 2024 10:21
@robmry robmry requested a review from thaJeztah October 24, 2024 10:21
thaJeztah
thaJeztah approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

daemon/info_windows.go
Comment on lines +14 to +17
if _, ok := cfg.Features["windows-dns-proxy"]; ok {
v.Warnings = append(v.Warnings, `
WARNING: Feature flag "windows-dns-proxy" has been removed, forwarding to external DNS resolvers is enabled.`)
}
Copy link
Member

@thaJeztah thaJeztah Oct 24, 2024

Choose a reason for hiding this comment

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

Thanks! Yes, I was thinking we'd need it only if someone has false set, but I guess this is good as well to tell them "remove this from your config"

We can remove this in v29.0 or so (maybe earlier, it's just a warning)

@thaJeztah thaJeztah merged commit 281bdbd into moby:master Oct 24, 2024
@robmry robmry deleted the 47732_remove_windows-dns-proxy_featflag branch October 25, 2024 10:49
@thaJeztah thaJeztah mentioned this pull request Feb 17, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@robmry robmry

Labels

area/networking/dns Networking area/networking Networking impact/changelog kind/refactor PR's that refactor, or clean-up code platform/windows

Projects

None yet

Milestone

28.0.0

Development

Successfully merging this pull request may close these issues.

Remove windows-dns-proxy feature flag

2 participants

@robmry @thaJeztah