Skip to content

Fork CRI server for Sandbox API integration work#7164

Merged
fuweid merged 4 commits intocontainerd:mainfrom
mxpv:cri-fork
Jul 15, 2022
Merged

Fork CRI server for Sandbox API integration work#7164
fuweid merged 4 commits intocontainerd:mainfrom
mxpv:cri-fork

Conversation

@mxpv
Copy link
Member

@mxpv mxpv commented Jul 13, 2022

This forks cri/server -> cri/sbserver package to unblock Sandbox CRI integration. This will allow us to iterate faster and don't worry about affecting existing CRI users.

Because we haven't figured out how exactly we'll be switching between CRI implementations (either an option in the CRI config or runtime or something else), I've added ENABLE_CRI_SANDBOXES env variable, which makes it really easy to enable integration tests.

mxpv added 3 commits July 13, 2022 10:54
Signed-off-by: Maksym Pavlenko <[email protected]>
panic: duplicate metrics collector registration attempted

Signed-off-by: Maksym Pavlenko <[email protected]>
@mxpv mxpv requested a review from mikebrow July 13, 2022 21:45
} else if isLocalHost(host) && u.Scheme == "http" {
// Skipping TLS verification for localhost
transport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.
@mxpv
Copy link
Member Author

mxpv commented Jul 13, 2022

/test pull-containerd-node-e2e

@containerd containerd deleted a comment from k8s-ci-robot Jul 13, 2022
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.

a couple comments about the new env variable..

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
per slack discussion will remove support for the environment setup as soon as we have some config toml to replace it.. (before r.next)

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Basically test it in local and it works well. Agree to use config to enable sbserver instead of env.

And we also update some integration cases in follow-up.

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