Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YUNIKORN-2135] Add integration test code coverage #733

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jobs:
uses: codecov/codecov-action@v3
with:
files: build/coverage.txt
flags: unittests

e2e-tests:
needs: build
Expand Down Expand Up @@ -59,3 +60,46 @@ jobs:
env:
KIND_NODE_IMAGE: ${{ matrix.k8s }}
KIND_EXTRA_ARGS: ${{ matrix.plugin }}
- name: Archive code coverage results
uses: actions/upload-artifact@v3
with:
name: go-cover-dir-merged
path: go-cover-dir-merged/
if-no-files-found: 'ignore'

e2e-tests-code-coverage:
needs: e2e-tests
runs-on: ubuntu-latest
steps:
- name: Checkout source code
uses: actions/checkout@v3
with:
fetch-depth: 2
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version-file: .go_version
- name: Download code-coverage-report artifacts
uses: actions/download-artifact@v3
with:
path: downloaded_artifacts
- run: |
if [ -d downloaded_artifacts/go-cover-dir-merged ];
then
if [ "$(ls -A downloaded_artifacts/go-cover-dir-merged)" ];
then
go tool covdata textfmt -i=downloaded_artifacts/go-cover-dir-merged -o yunikorn-k8shim.txt -pkg=github.com/apache/yunikorn-k8shim/...
go tool covdata textfmt -i=downloaded_artifacts/go-cover-dir-merged -o yunikorn-core.txt -pkg=github.com/apache/yunikorn-core/...
else
echo "No files found in downloaded_artifacts/go-cover-dir-merged folder. Skip the rest of the steps."
exit 0
fi
else
echo "No downloaded_artifacts/go-cover-dir-merged folder found. Skip the rest of the steps."
exit 0
fi
- name: Code coverage
uses: codecov/codecov-action@v3
with:
files: yunikorn-k8shim.txt
flags: e2e
1 change: 1 addition & 0 deletions .github/workflows/push-master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ jobs:
uses: codecov/codecov-action@v3
with:
files: build/coverage.txt
flags: unittests
20 changes: 14 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ endif
# Make sure we are in the same directory as the Makefile
BASE_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))

# Set cover flags for go build and uid/gid for dockerfile
ifeq ($(ENABLE_GO_COVER_DIR), TRUE)
DOCKERFILE_UID_ARG := --build-arg UID=0
DOCKERFILE_GID_ARG := --build-arg GID=0
COVER_FLAGS := -cover -coverpkg=github.com/apache/yunikorn-k8shim/...,github.com/apache/yunikorn-core/...
ADMISSION_COVER_FLAGS := -cover -coverpkg=github.com/apache/yunikorn-k8shim/...
endif

# Output directories
OUTPUT=build
DEV_BIN_DIR=${OUTPUT}/dev
Expand Down Expand Up @@ -340,7 +348,7 @@ $(RELEASE_BIN_DIR)/$(SCHEDULER_BINARY): go.mod go.sum $(shell find pkg)
-ldflags '-extldflags "-static" -X ${FLAG_PREFIX}.buildVersion=${VERSION} -X ${FLAG_PREFIX}.buildDate=${DATE} -X ${FLAG_PREFIX}.isPluginVersion=false -X ${FLAG_PREFIX}.goVersion=${GO_VERSION} -X ${FLAG_PREFIX}.arch=${EXEC_ARCH} -X ${FLAG_PREFIX}.coreSHA=${CORE_SHA} -X ${FLAG_PREFIX}.siSHA=${SI_SHA} -X ${FLAG_PREFIX}.shimSHA=${SHIM_SHA}' \
-tags netgo \
-installsuffix netgo \
./pkg/cmd/shim/
${COVER_FLAGS} ./pkg/cmd/shim/

# Build plugin binary in a production ready version
.PHONY: plugin
Expand All @@ -356,7 +364,7 @@ $(RELEASE_BIN_DIR)/$(PLUGIN_BINARY): go.mod go.sum $(shell find pkg)
-ldflags '-extldflags "-static" -X ${FLAG_PREFIX}.buildVersion=${VERSION} -X ${FLAG_PREFIX}.buildDate=${DATE} -X ${FLAG_PREFIX}.isPluginVersion=true -X ${FLAG_PREFIX}.goVersion=${GO_VERSION} -X ${FLAG_PREFIX}.arch=${EXEC_ARCH} -X ${FLAG_PREFIX}.coreSHA=${CORE_SHA} -X ${FLAG_PREFIX}.siSHA=${SI_SHA} -X ${FLAG_PREFIX}.shimSHA=${SHIM_SHA}' \
-tags netgo \
-installsuffix netgo \
./pkg/cmd/schedulerplugin/
${COVER_FLAGS} ./pkg/cmd/schedulerplugin/

# Build a scheduler image based on the production ready version
.PHONY: sched_image
Expand All @@ -366,7 +374,7 @@ sched_image: scheduler docker/scheduler
@mkdir -p "$(DOCKER_DIR)/scheduler"
@cp -a "docker/scheduler/." "$(DOCKER_DIR)/scheduler/."
@cp "$(RELEASE_BIN_DIR)/$(SCHEDULER_BINARY)" "$(DOCKER_DIR)/scheduler/."
DOCKER_BUILDKIT=1 docker build \
DOCKER_BUILDKIT=1 docker build ${DOCKERFILE_UID_ARG} ${DOCKERFILE_GID_ARG} \
"$(DOCKER_DIR)/scheduler" \
-t "$(SCHEDULER_TAG)" \
--platform "linux/${DOCKER_ARCH}" \
Expand All @@ -386,7 +394,7 @@ plugin_image: plugin docker/plugin conf/scheduler-config.yaml
@cp -a "docker/plugin/." "$(DOCKER_DIR)/plugin/."
@cp "$(RELEASE_BIN_DIR)/$(PLUGIN_BINARY)" "$(DOCKER_DIR)/plugin/."
@cp conf/scheduler-config.yaml "$(DOCKER_DIR)/plugin/scheduler-config.yaml"
DOCKER_BUILDKIT=1 docker build \
DOCKER_BUILDKIT=1 docker build ${DOCKERFILE_UID_ARG} ${DOCKERFILE_GID_ARG} \
"$(DOCKER_DIR)/plugin" \
-t "$(PLUGIN_TAG)" \
--platform "linux/${DOCKER_ARCH}" \
Expand All @@ -411,7 +419,7 @@ $(RELEASE_BIN_DIR)/$(ADMISSION_CONTROLLER_BINARY): go.mod go.sum $(shell find pk
-ldflags '-extldflags "-static" -X ${FLAG_PREFIX}.buildVersion=${VERSION} -X ${FLAG_PREFIX}.buildDate=${DATE} -X ${FLAG_PREFIX}.goVersion=${GO_VERSION} -X ${FLAG_PREFIX}.arch=${EXEC_ARCH}' \
-tags netgo \
-installsuffix netgo \
./pkg/cmd/admissioncontroller
${ADMISSION_COVER_FLAGS} ./pkg/cmd/admissioncontroller

# Build an admission controller image based on the production ready version
.PHONY: adm_image
Expand All @@ -421,7 +429,7 @@ adm_image: admission docker/admission
@mkdir -p "$(DOCKER_DIR)/admission"
@cp -a "docker/admission/." "$(DOCKER_DIR)/admission/."
@cp "$(RELEASE_BIN_DIR)/$(ADMISSION_CONTROLLER_BINARY)" "$(DOCKER_DIR)/admission/."
DOCKER_BUILDKIT=1 docker build \
DOCKER_BUILDKIT=1 docker build ${DOCKERFILE_UID_ARG} ${DOCKERFILE_GID_ARG} \
"$(DOCKER_DIR)/admission" \
-t "$(ADMISSION_TAG)" \
--platform "linux/${DOCKER_ARCH}" \
Expand Down
6 changes: 5 additions & 1 deletion docker/admission/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

ARG UID=4444
ARG GID=4444

FROM --platform=$TARGETPLATFORM scratch
COPY --chown=0:0 yunikorn-admission-controller /
USER 4444:4444
USER ${UID}:${GID}
ENTRYPOINT [ "/yunikorn-admission-controller" ]
6 changes: 5 additions & 1 deletion docker/plugin/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

ARG UID=4444
ARG GID=4444

FROM --platform=$TARGETPLATFORM scratch
COPY --chown=0:0 yunikorn-scheduler-plugin scheduler-config.yaml /
USER 4444:4444
USER ${UID}:${GID}
ENTRYPOINT [ "/yunikorn-scheduler-plugin", "--bind-address=0.0.0.0", "--config=/scheduler-config.yaml" ]
6 changes: 5 additions & 1 deletion docker/scheduler/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

ARG UID=4444
ARG GID=4444

FROM --platform=$TARGETPLATFORM scratch
COPY --chown=0:0 yunikorn-scheduler /
USER 4444:4444
USER ${UID}:${GID}
ENTRYPOINT [ "/yunikorn-scheduler" ]
40 changes: 37 additions & 3 deletions scripts/run-e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ function install_cluster() {
if [ "${GIT_CLONE}" = "true" ]; then
check_cmd "git"
rm -rf ./build/yunikorn-release
git clone --depth 1 https://github.com/apache/yunikorn-release.git ./build/yunikorn-release
# TODO: update following branch if PR in yunikorn-release is merged
git clone --depth 1 https://github.com/FrankYang0529/yunikorn-release.git -b YUNIKORN-2135 ./build/yunikorn-release
Copy link
Contributor

Choose a reason for hiding this comment

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

As indicated in the yunikorn-release repo PR, I think we should find a way to deploy this without polluting the upstream helm charts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the approach we should use: https://helm.sh/docs/topics/advanced/#post-rendering

Helm supports a post-renderer that can be used to update the generated output. We can use this in the e2e test script during install to "patch" the installed charts with what is required here. An ideal post-renderer would be kustomize (see https://kustomize.io/). An example using the two together can be found here: https://github.com/thomastaylor312/advanced-helm-demos/tree/master/post-render

We can setup a kustomize script to patch only the objects we need so that the original helm charts can stay pristine. All that should be required is an ENV var and build-mount for /tmp/coverage (or something like it).

fi
if [ ! -d "${CHART_PATH}" ]; then
exit_on_error "helm charts not found in path: ${CHART_PATH}"
Expand All @@ -96,7 +97,7 @@ function install_cluster() {
# build docker images from latest code, so that we can install yunikorn with these latest images
echo "step 3/7: building docker images from latest code"
check_docker
QUIET="--quiet" REGISTRY=local VERSION=latest make image
QUIET="--quiet" REGISTRY=local VERSION=latest ENABLE_GO_COVER_DIR=TRUE make image
exit_on_error "build docker images failed"
QUIET="--quiet" REGISTRY=local VERSION=latest make webtest_image
exit_on_error "build test web images failed"
Expand Down Expand Up @@ -136,14 +137,45 @@ function install_cluster() {
--set admissionController.image.pullPolicy=IfNotPresent \
--set web.image.repository=local/yunikorn \
--set web.image.tag="${WEBTEST_IMAGE}" \
--set web.image.pullPolicy=IfNotPresent
--set web.image.pullPolicy=IfNotPresent \
--set enableGoCoverDir=true
exit_on_error "failed to install yunikorn"
"${KUBECTL}" wait --for=condition=available --timeout=300s deployment/yunikorn-scheduler -n yunikorn
exit_on_error "failed to wait for yunikorn scheduler deployment being deployed"
"${KUBECTL}" wait --for=condition=ready --timeout=300s pod -l app=yunikorn -n yunikorn
exit_on_error "failed to wait for yunikorn scheduler pods being deployed"
}

function uninstall_yunikorn() {
echo "uninstall yunikorn"
install_tools
"${HELM}" uninstall yunikorn --namespace yunikorn --wait --cascade foreground
exit_on_error "failed to uninstall yunikorn"
}

function copy_go_cover_dir() {
echo "copy coverage profile files from kind containers to local go-cover-dir-merged directory"
install_tools
mkdir -p go-cover-dir-merged
GO_COVER_DIR_FOLDERS=()
for i in $("${KIND}" get nodes --name "${CLUSTER_NAME}"); do
if docker exec -i "$i" test -d /go-cover-dir
then
docker cp "$i":/go-cover-dir "$i"
GO_COVER_DIR_FOLDERS+=("$i")
fi
done

if [ ${#GO_COVER_DIR_FOLDERS[@]} -ne 0 ]; then
GO_COVER_DIR_STR=$(printf ",%s" "${GO_COVER_DIR_FOLDERS[@]}")
GO_COVER_DIR_STR=${GO_COVER_DIR_STR:1}
go tool covdata merge -i="${GO_COVER_DIR_STR}" -o go-cover-dir-merged
for i in "${GO_COVER_DIR_FOLDERS[@]}"; do
rm -rf "$i"
done
fi
}

function delete_cluster() {
echo "deleting K8s cluster: ${CLUSTER_NAME}"
install_tools
Expand Down Expand Up @@ -269,6 +301,8 @@ if [ "${ACTION}" == "test" ]; then
fi
make e2e_test
exit_on_error "e2e tests failed"
uninstall_yunikorn
copy_go_cover_dir
elif [ "${ACTION}" == "install" ]; then
check_cmd "${GO}"
check_opt "kind-node-image-version" "${CLUSTER_VERSION}"
Expand Down