Skip to content

Conversation

@AkihiroSuda
Copy link
Member

These deprecations were mentioned in pkg/cri/config/config.go but not mentioned in RELEASES.md.

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@AkihiroSuda AkihiroSuda added this to the 1.7 milestone Mar 9, 2023
@AkihiroSuda AkihiroSuda force-pushed the deprecation-20230309 branch from ff2238c to 2e2c848 Compare March 9, 2023 03:19
// Configs are configs for each registry.
// The key is the domain name or IP of the registry.
// This option will be fully deprecated for ConfigPath in the future.
// DEPRECATED: Use ConfigPath instead.
Copy link
Member Author

Choose a reason for hiding this comment

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

(This has been already deprecated since v1.5)

##### Deprecation of `registry.mirrors` and `registry.configs` in CRI plugin

// today, who don't have a cni daemonset in production. NetworkPluginConfTemplate is
// a temporary backward-compatible solution for them.
// TODO(random-liu): Deprecate this option when kubenet is deprecated.
// DEPRECATED: use CNI configs
Copy link
Member Author

Choose a reason for hiding this comment

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

Substantially we have been deprecating this for years, but looks like we have forgotten to formally deprecate it

Copy link
Contributor

@BenTheElder BenTheElder May 30, 2023

Choose a reason for hiding this comment

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

Commented on #8596. It's unfortunate this was ostensibly deprecated for so long without any notice, I don't think a reasonable replacement is readily available yet. This allowed for very simple generic networking using the Kubernetes built in node pod CIDR IPAM, avoiding the need to fetch the node object through some external process (which has scalability, auth, complexity issues).

Copy link
Contributor

@aojea aojea May 30, 2023

Choose a reason for hiding this comment

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

This TODO is from a time where this field was going to be deprecated and made sense at that point in time kubernetes/kubernetes#57130

However, the time evolved and showed to be useful and there are several projects dependent on it , as a proof you can see that the kubernetes field got added new features instead of being deprecated as you can see in this KEP https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/2593-multiple-cluster-cidrs

So, it seems that happened here is that we forget to tell containerd how useful is this field and this TODO should have been removed

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, let's de-deprecate this

@AkihiroSuda AkihiroSuda force-pushed the deprecation-20230309 branch 3 times, most recently from 17f1685 to de3f80b Compare March 9, 2023 03:55
@AkihiroSuda AkihiroSuda marked this pull request as ready for review March 9, 2023 04:10
RELEASES.md Outdated
|`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*]` | `runtime_engine` | containerd v1.3 | containerd v2.0 | Use runtime v2 |
|`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*]` | `runtime_root` | containerd v1.3 | containerd v2.0 | Use `options.Root` |
|`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*.options]` | `CriuPath` | containerd v1.7 | containerd v2.0 | Set `$PATH` to the `criu` binary |
|`[plugins."io.containerd.grpc.v1.cri".registry]` | `auths` | containerd v1.3 | containerd v2.0 | Use [`config_path`](./docs/hosts.md) |
Copy link
Member

Choose a reason for hiding this comment

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

config_path does not have support for authentication and there is no plan to have it supported in this way. Recommendation would be use pull secrets or could link to an in progress issue for 2.0.

Copy link
Member

Choose a reason for hiding this comment

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

Could link to this issue #8228 for now

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@AkihiroSuda AkihiroSuda force-pushed the deprecation-20230309 branch from de3f80b to 3d48d9e Compare March 9, 2023 06:02
These deprecations were mentioned in `pkg/cri/config/config.go`
but not mentioned in `RELEASES.md`.

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda force-pushed the deprecation-20230309 branch from 3d48d9e to 625217d Compare March 9, 2023 06:13
@AkihiroSuda
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants