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

Test containerd repo #673

Merged
merged 2 commits into from
Mar 16, 2018
Merged

Conversation

Random-Liu
Copy link
Member

This PR:

  1. Run make sync-vendor to sync vendors with containerd.
  2. Break install-deps.sh into install-cni.sh, install-crictl.sh, install-runc.sh and install-containerd.sh.
  3. Create test/build-containerd. It uses install-containerd.sh to build containerd binaries from existing containerd repo (test-infra will checkout containerd repo for containerd repo CI) into release tarball for test.

In this way, we can build CI test for containerd repo.

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.

looks good to me just a couple nit comments to address

# BUILDTAGS are bulid tags for runc and containerd.
BUILDTAGS=${BUILDTAGS:-seccomp apparmor}

CONTAINERD_DIR=${DESTDIR}/usr/local
Copy link
Member

Choose a reason for hiding this comment

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

Suggest moving all these component specific DIR and PKG variable declarations over to the component install script just after they source this util script.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. Will do.

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

TMPGOPATH=$(mktemp -d /tmp/cri-install-crictl.XXXX)
GOPATH=${TMPGOPATH}

#Install crictl
Copy link
Member

Choose a reason for hiding this comment

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

since we've split this into it's own install.. and moved the from-vendor calls to each separate install.. let's also go ahead and move the CRITOOL_ variable assignments from utils.sh to here...

Copy link
Member Author

@Random-Liu Random-Liu Mar 16, 2018

Choose a reason for hiding this comment

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

Actually CRITOOL_VERSION is also used in make test-cri. And I don't want to include critest in install-deps.sh for now, because that binary requires cri-tools repo to run test.

Although @feiskyer built everything into one binary recently kubernetes-sigs/cri-tools#261, it still doesn't support running test in parallel.

I think after single binary critest is fully done, we can build and install critest here. And remove the critest installation in make test-cri.

@Random-Liu Random-Liu force-pushed the test-containerd-repo branch from be9fe91 to a69f355 Compare March 16, 2018 17:51
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

@mikebrow mikebrow added the lgtm label Mar 16, 2018
@Random-Liu Random-Liu merged commit 65b5240 into containerd:master Mar 16, 2018
@Random-Liu Random-Liu deleted the test-containerd-repo branch March 16, 2018 20:04
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.

3 participants