Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Support CNI DNS capabilities. #1244

Merged

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Aug 22, 2019

For #1257.

Fixes #1243.
Depends on containerd/go-cni#48.

This is required for Windows container support as well. (See kubernetes/kubernetes#67435)

/cc @yujuhong

Signed-off-by: Lantao Liu [email protected]

@Random-Liu Random-Liu added this to the v1.4 milestone Aug 22, 2019
@Random-Liu Random-Liu force-pushed the support-cni-dns-capabilities branch from 531a004 to 203ffc9 Compare August 22, 2019 17:20
Copy link
Contributor

@jterry75 jterry75 left a comment

Choose a reason for hiding this comment

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

LGTM

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
just a couple small nits

}

// Will return an error if the bandwidth limitation has the wrong unit
// or an unreasonable valure see validateBandwidthIsReasonable()
Copy link
Member

Choose a reason for hiding this comment

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

s/valure/value/

"IgnoreUnknown": "1",
}
}

// toCNIBandWidth converts CRI annotations to CNI bandwidth.
func toCNIBandWidth(annotations map[string]string) (*cni.BandWidth, error) {
ingress, egress, err := bandwidth.ExtractPodBandwidthResources(annotations)
if err != nil {
return nil, errors.Errorf("reading pod bandwidth annotations: %v", err)
Copy link
Member

Choose a reason for hiding this comment

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

errors.Wrap

@Random-Liu Random-Liu force-pushed the support-cni-dns-capabilities branch from 203ffc9 to 28aef2f Compare August 22, 2019 21:29
@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 22, 2019
@k8s-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@Random-Liu
Copy link
Member Author

Addressed @mikebrow's comment.

Apply lgtm based on #1244 (review)

@Random-Liu Random-Liu merged commit 6dc2a87 into containerd:master Aug 22, 2019
@Random-Liu Random-Liu deleted the support-cni-dns-capabilities branch August 22, 2019 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support new CNI capabilities.
4 participants