Skip to content

pkg/cri/config: remove deprecated Auths (auths)#7526

Closed
thaJeztah wants to merge 1 commit intocontainerd:mainfrom
thaJeztah:cri_remove_deprecated
Closed

pkg/cri/config: remove deprecated Auths (auths)#7526
thaJeztah wants to merge 1 commit intocontainerd:mainfrom
thaJeztah:cri_remove_deprecated

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 13, 2022 •
edited
Loading

pkg/cri/config: remove deprecated Auths (auths)

This was marked to be removed for containerd v1.6.0

estesp
estesp approved these changes Oct 14, 2022
View reviewed changes
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah force-pushed the cri_remove_deprecated branch from 6148e5e to a981657 Compare October 16, 2022 22:56
@samuelkarp
Copy link
Member

samuelkarp commented Oct 17, 2022

Should we wait until 2.0 to remove these?

/retest

@jterry75
Copy link
Contributor

jterry75 commented Oct 17, 2022

@kevpar - FYI

@jterry75
Copy link
Contributor

jterry75 commented Oct 17, 2022

LGTM! TY

@thaJeztah thaJeztah force-pushed the cri_remove_deprecated branch from a981657 to a7ad300 Compare October 20, 2022 07:27
@AkihiroSuda
Copy link
Member

AkihiroSuda commented Oct 24, 2022

Looks like we want to keep compatibility until v2.0

#7575

@AkihiroSuda AkihiroSuda added this to the 2.0 milestone Oct 24, 2022
@dmcgowan dmcgowan added the status/needs-major-release Can only be accepted into a major release label Nov 30, 2022
@Iceber
Copy link
Member

Iceber commented May 17, 2023

@thaJeztah HI, This pr needs rebase and update Release.md

containerd/RELEASES.md

Line 396 in 8c21759

|`[plugins."io.containerd.grpc.v1.cri".registry]` | `auths` | containerd v1.3 | containerd v2.0 | Use [`ImagePullSecrets`](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/). See also [#8228](https://github.com/containerd/containerd/issues/8228). |

DefaultRuntime and UntrustedWorkloadRuntime have been removed, auths has not been removed

@Iceber Iceber mentioned this pull request May 19, 2023
Closed
mikebrow
mikebrow reviewed May 19, 2023
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

looking good .. see https://github.com/containerd/containerd/blob/main/docs/cri/config.md?plain=1 for references in the docs to these fields

@lengrongfu lengrongfu mentioned this pull request May 26, 2023
Closed
@mxpv mxpv added the status/needs-update Awaiting contributor update label Sep 12, 2023
@samuelkarp
Copy link
Member

samuelkarp commented Nov 9, 2023

@thaJeztah Are we ready for this to go in? If so, it needs a rebase.

@thaJeztah thaJeztah force-pushed the cri_remove_deprecated branch from a7ad300 to 2dec36d Compare December 4, 2023 13:47
@thaJeztah thaJeztah changed the title pkg/cri/config: remove deprecated config options pkg/cri/config: remove deprecated Auths (auths) Dec 4, 2023
@thaJeztah
pkg/cri/config: remove deprecated Auths (auths)
78bae3a 
This was marked to be removed for containerd v1.6.0

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the cri_remove_deprecated branch from 2dec36d to 78bae3a Compare December 4, 2023 13:50
@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 4, 2023

Rebased; only the last commit was still needed; also updated RELEASES.md to reflect that it was removed

@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 4, 2023

And looks like there's two remaining options to be removed; I'll have a look at doing a follow-up.

@AkihiroSuda AkihiroSuda removed the status/needs-update Awaiting contributor update label Dec 4, 2023
mikebrow
mikebrow approved these changes Dec 4, 2023
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM..
for the follow up.. we also have auth refs here to remove:
https://github.com/containerd/containerd/blame/main/docs/cri/registry.md#L15

mikebrow
mikebrow reviewed Dec 4, 2023
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

..

@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 4, 2023

ah, thanks; didn't check docs. I can remove that as part of this PR

@mikebrow
Copy link
Member

mikebrow commented Dec 4, 2023

as an alternative you could just say removed from v2.. tricky to have docs in a main branch that might be read for prior releases..

samuelkarp
samuelkarp reviewed Dec 5, 2023
View reviewed changes
pkg/cri/config/config_test.go
},
},
},
warnings: []deprecation.Warning{deprecation.CRIRegistryAuths},
Copy link
Member

@samuelkarp samuelkarp Dec 5, 2023

Choose a reason for hiding this comment

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

You can remove this from the deprecation package too.

@dmcgowan
Copy link
Member

dmcgowan commented Dec 6, 2023

I don't think we have the preconditions to merge this one yet However, it will not be removed until a suitable secret management alternative is available as a plugin. as ImagePullSecrets is not a suitable alternative for host managed auth.

@k8s-ci-robot
Copy link

k8s-ci-robot commented Jan 12, 2024

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dims dims added the area/cri Container Runtime Interface (CRI) label Feb 7, 2024
@mikebrow mikebrow mentioned this pull request Feb 8, 2024
Closed
2 tasks
@dmcgowan
Copy link
Member

dmcgowan commented Apr 25, 2024

Going to close this one, these properties will get migrated in a later version. Many users expressed that image pull secrets is not a suitable alternative for their configuration. See #9966

@dmcgowan dmcgowan closed this Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelkarp samuelkarp samuelkarp left review comments

@mikebrow mikebrow mikebrow approved these changes

@mxpv mxpv mxpv approved these changes

@estesp estesp estesp approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

area/cri Container Runtime Interface (CRI) needs-rebase size/M status/needs-major-release Can only be accepted into a major release

Projects

Pull Request Review
Archived in project

Milestone

2.0

Development

Successfully merging this pull request may close these issues.