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

Move build.sh into the sdk binary #541

Closed
AlexNPavel opened this issue Sep 26, 2018 · 1 comment
Closed

Move build.sh into the sdk binary #541

AlexNPavel opened this issue Sep 26, 2018 · 1 comment
Assignees

Comments

@AlexNPavel
Copy link
Contributor

AlexNPavel commented Sep 26, 2018

The build.sh script is mostly unnecessary and does not provide any benefits over handing the building from the operator-sdk's binary. In addition, any updates that the sdk gets will not be reflected in existing project as build.sh is generated on new. Currently, this will cause issues for people created a project before the --enable-tests flag was added to the sdk and then try to use --enable-tests.

In addition, we should look into the way that dockerfiles are managed in cases like this. Currently, the above situation (trying to build an image with tests on a project that was generated before that was supported) will not work even with the build script merge because it will be missing the test-framework dockerfile. For now, we can just check if it exists and generate it if it doesn't, but in the long-term, we should find a better solution for managing these files.

@hasbro17
Copy link
Contributor

Elaborating on the current issue.
Before #469 operator-sdk build would run the build script tmp/build/build.sh

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

if ! which go > /dev/null; then
	echo "golang needs to be installed"
	exit 1
fi

BIN_DIR="$(pwd)/tmp/_output/bin"
mkdir -p ${BIN_DIR}
PROJECT_NAME="memcached-operator"
REPO_PATH="github.com/operator-framework/operator-sdk-samples/memcached-operator"
BUILD_PATH="${REPO_PATH}/cmd/${PROJECT_NAME}"
echo "building "${PROJECT_NAME}"..."
GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o ${BIN_DIR}/${PROJECT_NAME} $BUILD_PATH

But now with operator-sdk build --enable-tests it expects the script to build the test binary as well:

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

if ! which go > /dev/null; then
	echo "golang needs to be installed"
	exit 1
fi

BIN_DIR="$(pwd)/tmp/_output/bin"
mkdir -p ${BIN_DIR}
PROJECT_NAME="memcached-operator"
REPO_PATH="github.com/operator-framework/operator-sdk-samples/memcached-operator"
TEST_PATH="${REPO_PATH}/${TEST_LOCATION}"
BUILD_PATH="${REPO_PATH}/cmd/${PROJECT_NAME}"
echo "building "${PROJECT_NAME}"..."
GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o ${BIN_DIR}/${PROJECT_NAME} $BUILD_PATH
if $ENABLE_TESTS ; then
	echo "building "${PROJECT_NAME}-test"..."
	GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go test -c -o ${BIN_DIR}/${PROJECT_NAME}-test $TEST_PATH
fi

So for projects with the older script the operator-sdk build --enable-test command will fail. So we should stop generating helper scripts and just implement them as Go code in the binary.

I've already done the same for the codegen script tmp/codegen/update-generated.sh in the controller-runtime refactor branch.
5eb5814

AlexNPavel added a commit to AlexNPavel/operator-sdk that referenced this issue Oct 2, 2018
This commit moves all functionality of build.sh into the build.go
file to embed the functionality into the binary. It also allows the
binary to generate the files needed when making tests (Dockerfile
and go-test.sh) in case someone tries to build testing enabled
images using a project made before that functionality was added.

See operator-framework#541
AlexNPavel added a commit that referenced this issue Oct 2, 2018
* commands/{build,cmdutil},pkg/generator: remove build.sh

This commit moves all functionality of build.sh into the build.go
file to embed the functionality into the binary. It also allows the
binary to generate the files needed when making tests (Dockerfile
and go-test.sh) in case someone tries to build testing enabled
images using a project made before that functionality was added.

See #541
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

No branches or pull requests

2 participants