Skip to content
Closed
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
10 changes: 10 additions & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,16 @@ All e2e resources (host operator, member operator, registration-service, CRDs, e

* `make dev-deploy-e2e-local` - deploys the same resources as `make test-e2e-local` in dev environment but doesn't run tests.

* `make dev-deploy-e2e-local-debug` - deploys the same resources as `make test-e2e-local` in the local OpenShift
instance, but builds the services and the operators without any code optimizations. It also ships Delve in the images,
and launches those services with it, which leaves the debugger listening on port `50000`. The default architecture for
which the binaries are built are `linux-amd64`, but you can override it with `export TARGET_ARCH=${YOUR_ARCH}`. Then,
you can simply `oc port-forward ${POD} 50000:50000` and connect to them with your favorite IDE. The services that are
Comment thread
MikelAlejoBR marked this conversation as resolved.
built like that are:
** The registration service.
** The host operator controller manager.
** The member operator controller manager.
Comment thread
MikelAlejoBR marked this conversation as resolved.

* `make dev-deploy-e2e` - deploys the same resources as `make test-e2e` in dev environment but doesn't run tests.

* `make deploy-single-member-e2e-latest` - deploys the same resources (using the latest and greatest images of Toolchain operators) as `make test-e2e` but with only one member and doesn't run tests.
Expand Down
1 change: 1 addition & 0 deletions deploy/host-operator/dev/toolchainconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
deactivationDomainsExcluded: '@redhat.com'
registrationService:
environment: 'dev'
replicas: 1
members:
default:
autoscaler:
Expand Down
39 changes: 39 additions & 0 deletions make/dev.mk
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,45 @@ setup-dev-sso:
.PHONY: dev-deploy-e2e-local
dev-deploy-e2e-local: deploy-e2e-local-to-dev-namespaces print-reg-service-link

# Builds the services' and operators' images with the debugger in it, so that
# then an IDE can be connected to them. Since the targets down the line use
# the default namespaces, we can use them to patch the required CRs in order
# to launch the binaries with Delve.
.ONESHELL:
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

.ONESHELL is a global directive that affects all subsequent targets.

The .ONESHELL directive on Line 46 applies to all targets defined after it in this file, not just dev-deploy-e2e-local-debug. This can have unintended consequences:

  1. All subsequent targets will run their recipes in a single shell, which changes error propagation behavior.
  2. Other targets may not expect this behavior and could mask failures.

If .ONESHELL is needed only for variable persistence in the debug target (for HOST_CSV_NAME and MEMBER_CSV_NAME), consider moving it to a separate included file or restructuring the target to avoid the need for .ONESHELL.

♻️ Alternative approach

Instead of .ONESHELL, you could capture variables and pass them to a sub-shell:

-.ONESHELL:
 .PHONY: dev-deploy-e2e-local-debug
 dev-deploy-e2e-local-debug: export DEBUG_MODE=true
 dev-deploy-e2e-local-debug:
-	# $(MAKE) dev-deploy-e2e-local
-
-	# Get the CSVs for the host and member operators, in order to be able to
-	# patch them.
-	HOST_CSV_NAME=$$(oc get --namespace "${DEFAULT_HOST_NS}" --output name ClusterServiceVersion)
-	MEMBER_CSV_NAME=$$(oc get --namespace ${DEFAULT_MEMBER_NS} --output name ClusterServiceVersion)
-
-	@echo "CVSs are: $${HOST_CSV_NAME} and $${MEMBER_CSV_NAME}"
-	...
+	$(MAKE) dev-deploy-e2e-local
+	@bash -c ' \
+		set -e; \
+		HOST_CSV_NAME=$$(oc get --namespace "${DEFAULT_HOST_NS}" --output name ClusterServiceVersion); \
+		MEMBER_CSV_NAME=$$(oc get --namespace "${DEFAULT_MEMBER_NS}" --output name ClusterServiceVersion); \
+		echo "CSVs are: $$HOST_CSV_NAME and $$MEMBER_CSV_NAME"; \
+		...'

This limits the .ONESHELL-like behavior to only this target.

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@MikelAlejoBR this is needed to propagate the export DEBUG_MODE=true to all makefile targets that are transitively executed, correct?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry @MatousJobanek, with all the noise I missed your comments. Yes, you're right. That export helps leaving many things untouched, but I might have to give it a look again to see if I can avoid that somehow...

.PHONY: dev-deploy-e2e-local-debug
dev-deploy-e2e-local-debug: export DEBUG_MODE=true
Comment thread
MikelAlejoBR marked this conversation as resolved.
dev-deploy-e2e-local-debug:
$(MAKE) dev-deploy-e2e-local

# Get the CSVs for the host and member operators, in order to be able to
# patch them.
HOST_CSV_NAME=$$(oc get --namespace="${DEFAULT_HOST_NS}" --output name ClusterServiceVersion)
MEMBER_CSV_NAME=$$(oc get --namespace="${DEFAULT_MEMBER_NS}" --output name ClusterServiceVersion)

# Patch the host operator indicating which command the registration
# service should be run with. The command simply adds an environment
# variable to the operator which then makes sure that the registration
# service is run with Delve, in case the user wants to connect to it.
if ! oc get "$${HOST_CSV_NAME}" --output jsonpath="{.spec.install.spec.deployments[0].spec.template.spec.containers[1].env}" | grep -q "REGISTRATION_SERVICE_COMMAND"; then \
oc patch --namespace="${DEFAULT_HOST_NS}" "$${HOST_CSV_NAME}" --type='json' --patch='[{"op": "add", "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/1/env/-", "value": {"name": "REGISTRATION_SERVICE_COMMAND", "value": "[\"dlv\", \"--listen=:50000\", \"--headless\", \"--continue\", \"--api-version=2\", \"--accept-multiclient\", \"exec\", \"/usr/local/bin/registration-service\"]"}}]'

# Wait for the registration service's command to have the "dlv" bit, and the rollout for its deployment to be
# complete.
@echo "Waiting for the registration service's deployment to get updated..."
oc wait --namespace="${DEFAULT_HOST_NS}" --timeout=3m --for=jsonpath='{.spec.template.spec.containers[0].command[0]}'="dlv" "deployment/registration-service"
oc rollout status --namespace="${DEFAULT_HOST_NS}" --timeout=3m deployment/registration-service
fi

# Patch the host-operator and member-operator CSVs to make them run with
# Delve.
oc patch --namespace="${DEFAULT_HOST_NS}" "$${HOST_CSV_NAME}" --type='json' --patch='[{"op": "replace", "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/1/args", "value": []}, {"op": "replace", "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/1/command", "value": ["dlv", "--listen=:50000", "--headless", "--continue", "--api-version=2", "--accept-multiclient", "exec", "/usr/local/bin/host-operator", "--", "--health-probe-bind-address=:8081", "--metrics-bind-address=127.0.0.1:8080", "--leader-elect"]}]'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

very minor - not a big fan of those "long" patch commands in the make targets. An alternative could be to have those into dedicated patch.yaml files, but it's very minor and ok for now 👍

@echo "Waiting for the host operator to be ready..."
oc wait --namespace "${DEFAULT_HOST_NS}" "$${HOST_CSV_NAME}" --for=jsonpath='{.status.phase}'=Succeeded --timeout=300s

oc patch --namespace="${DEFAULT_MEMBER_NS}" "$${MEMBER_CSV_NAME}" --type='json' --patch='[{"op": "replace", "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/args", "value": []}, {"op": "replace", "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/command", "value": ["dlv", "--listen=:50000", "--headless", "--continue", "--api-version=2", "--accept-multiclient", "exec", "/usr/local/bin/member-operator", "--", "--health-probe-bind-address=:8081", "--metrics-bind-address=127.0.0.1:8080", "--leader-elect"]}]'
@echo "Waiting for the member operator to be ready..."
oc wait --namespace "${DEFAULT_MEMBER_NS}" "$${MEMBER_CSV_NAME}" --for=jsonpath='{.status.phase}'=Succeeded --timeout=300s

.PHONY: dev-deploy-e2e-local-two-members
dev-deploy-e2e-local-two-members: deploy-e2e-local-to-dev-namespaces-two-members print-reg-service-link

Expand Down
14 changes: 12 additions & 2 deletions make/test.mk
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ ifneq (${FORCED_TAG},"")
$(eval FORCED_TAG_PARAM = -ft ${FORCED_TAG})
endif
endif
ifneq (${DEBUG_MODE},"")
ifneq (${DEBUG_MODE},)
$(eval DEBUG_MODE_PARAM = --debug-mode)
endif
endif
ifeq ($(DEPLOY_LATEST),true)
@echo "Installing latest version of the member-operator in namespace ${MEMBER_NS}"
${KSCTL_BIN_DIR}ksctl adm install-operator member --kubeconfig "$(or ${KUBECONFIG}, ${HOME}/.kube/config)" --namespace ${MEMBER_NS} ${KSCTL_INSTALL_TIMEOUT_PARAM} -y
Expand All @@ -340,7 +345,7 @@ ifeq ($(DEPLOY_LATEST),true)
endif
else
@echo "Installing specific version of the member-operator"
scripts/ci/manage-member-operator.sh -po ${PUBLISH_OPERATOR} -io ${INSTALL_OPERATOR} -mn ${MEMBER_NS} ${MEMBER_REPO_PATH_PARAM} -qn ${QUAY_NAMESPACE} -ds ${DATE_SUFFIX} ${MEMBER_NS_2_PARAM} ${FORCED_TAG_PARAM}
scripts/ci/manage-member-operator.sh -po ${PUBLISH_OPERATOR} -io ${INSTALL_OPERATOR} -mn ${MEMBER_NS} ${MEMBER_REPO_PATH_PARAM} -qn ${QUAY_NAMESPACE} -ds ${DATE_SUFFIX} ${MEMBER_NS_2_PARAM} ${FORCED_TAG_PARAM} ${DEBUG_MODE_PARAM}
endif

.PHONY: get-and-publish-host-operator
Expand All @@ -360,12 +365,17 @@ ifneq (${FORCED_TAG},"")
$(eval FORCED_TAG_PARAM = -ft ${FORCED_TAG})
endif
endif
ifneq (${DEBUG_MODE},"")
ifneq (${DEBUG_MODE},)
$(eval DEBUG_MODE_PARAM = --debug-mode)
endif
endif
ifeq ($(DEPLOY_LATEST),true)
@echo "Installing latest version of the host-operator"
${KSCTL_BIN_DIR}ksctl adm install-operator host --kubeconfig "$(or ${KUBECONFIG}, ${HOME}/.kube/config)" --namespace ${HOST_NS} ${KSCTL_INSTALL_TIMEOUT_PARAM} -y
else
@echo "Installing specific version of the host-operator"
scripts/ci/manage-host-operator.sh -po ${PUBLISH_OPERATOR} -io ${INSTALL_OPERATOR} -hn ${HOST_NS} ${HOST_REPO_PATH_PARAM} -ds ${DATE_SUFFIX} -qn ${QUAY_NAMESPACE} ${REG_REPO_PATH_PARAM} ${FORCED_TAG_PARAM}
scripts/ci/manage-host-operator.sh -po ${PUBLISH_OPERATOR} -io ${INSTALL_OPERATOR} -hn ${HOST_NS} ${HOST_REPO_PATH_PARAM} -ds ${DATE_SUFFIX} -qn ${QUAY_NAMESPACE} ${REG_REPO_PATH_PARAM} ${FORCED_TAG_PARAM} ${DEBUG_MODE_PARAM}
endif

###########################################################
Expand Down
9 changes: 7 additions & 2 deletions scripts/ci/manage-host-operator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ user_help () {
echo "-rr, --reg-repo-path Path to the registation service repo"
echo "-ds, --date-suffix Date suffix to be added to some resources that are created"
echo "-ft, --forced-tag Forces a tag to be set to all built images. In the case deployment the tag is used for index image in the created CatalogSource"
echo "-dm, --debug-mode Enables debug builds. The operator is built and pushed to quay with the Delve executable included in the image"
echo "-h, --help To show this help text"
echo ""
exit 0
Expand Down Expand Up @@ -68,6 +69,10 @@ read_arguments() {
FORCED_TAG=$1
shift
;;
-dm|--debug-mode)
DEBUG_MODE=true
shift
;;
*)
echo "$1 is not a recognized flag!" >> /dev/stderr
user_help
Expand Down Expand Up @@ -96,7 +101,7 @@ if [[ -n "${CI}${REG_REPO_PATH}${HOST_REPO_PATH}" ]] && [[ $(echo ${REPO_NAME} |
set_tags

if [[ ${PUBLISH_OPERATOR} == "true" ]]; then
push_image
push_image "${DEBUG_MODE}"
Comment thread
MikelAlejoBR marked this conversation as resolved.
REG_SERV_IMAGE_LOC=${IMAGE_LOC}
REG_REPO_PATH=${REPOSITORY_PATH}
fi
Expand All @@ -108,7 +113,7 @@ if [[ -n "${CI}${REG_REPO_PATH}${HOST_REPO_PATH}" ]] && [[ $(echo ${REPO_NAME} |
set_tags

if [[ ${PUBLISH_OPERATOR} == "true" ]]; then
push_image
push_image "${DEBUG_MODE}"
OPERATOR_IMAGE_LOC=${IMAGE_LOC}
make -C ${REPOSITORY_PATH} publish-current-bundle INDEX_IMAGE_TAG=${BUNDLE_AND_INDEX_TAG} BUNDLE_TAG=${BUNDLE_AND_INDEX_TAG} QUAY_NAMESPACE=${QUAY_NAMESPACE} OTHER_REPO_PATH=${REG_REPO_PATH} OTHER_REPO_IMAGE_LOC=${REG_SERV_IMAGE_LOC} IMAGE=${OPERATOR_IMAGE_LOC}
fi
Expand Down
7 changes: 6 additions & 1 deletion scripts/ci/manage-member-operator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ user_help () {
echo "-mr, --member-repo-path Path to the member operator repo"
echo "-ds, --date-suffix Date suffix to be added to some resources that are created"
echo "-ft, --forced-tag Forces a tag to be set to all built images. In the case deployment the tag is used for index image in the created CatalogSource"
echo "-dm, --debug-mode Enables debug builds. The operator is built and pushed to quay with the Delve executable included in the image"
echo "-h, --help To show this help text"
echo ""
exit 0
Expand Down Expand Up @@ -67,6 +68,10 @@ read_arguments() {
FORCED_TAG=$1
shift
;;
-dm|--debug-mode)
DEBUG_MODE=$1
shift
;;
*)
echo "$1 is not a recognized flag!" >> /dev/stderr
user_help
Expand Down Expand Up @@ -95,7 +100,7 @@ if [[ -n "${CI}${MEMBER_REPO_PATH}" ]] && [[ $(echo ${REPO_NAME} | sed 's/"//g')
set_tags

if [[ ${PUBLISH_OPERATOR} == "true" ]]; then
push_image
push_image "${DEBUG_MODE}"

OPERATOR_IMAGE_LOC=${IMAGE_LOC}
make -C ${REPOSITORY_PATH} publish-current-bundle INDEX_IMAGE_TAG=${BUNDLE_AND_INDEX_TAG} BUNDLE_TAG=${BUNDLE_AND_INDEX_TAG} QUAY_NAMESPACE=${QUAY_NAMESPACE} IMAGE=${OPERATOR_IMAGE_LOC}
Expand Down
14 changes: 12 additions & 2 deletions scripts/ci/manage-operator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,21 @@ set_tags() {
}

push_image() {
# When the "${DEBUG_MODE}" argument is passed, we instruct Make to push
# the "debug" images with Delve on them.
if [[ $1 == "true" ]]; then
DEBUG_MODE_SUFFIX="-debug"
else
DEBUG_MODE_SUFFIX=""
fi

# Set a default image builder in case none has been specified.
IMAGE_BUILDER="${IMAGE_BUILDER:-podman}"

GIT_COMMIT_ID=$(git --git-dir=${REPOSITORY_PATH}/.git --work-tree=${REPOSITORY_PATH} rev-parse --short HEAD)
IMAGE_LOC=quay.io/codeready-toolchain/${REPOSITORY_NAME}:${GIT_COMMIT_ID}
if is_provided_or_paired; then
IMAGE_BUILDER=${IMAGE_BUILDER:-"podman"}
make -C ${REPOSITORY_PATH} ${IMAGE_BUILDER}-push QUAY_NAMESPACE=${QUAY_NAMESPACE} IMAGE_TAG=${TAGS}
make -C ${REPOSITORY_PATH} ${IMAGE_BUILDER}-push${DEBUG_MODE_SUFFIX} QUAY_NAMESPACE=${QUAY_NAMESPACE} IMAGE_TAG=${TAGS}
IMAGE_LOC=quay.io/${QUAY_NAMESPACE}/${REPOSITORY_NAME}:${TAGS}
fi
}
Comment thread
MikelAlejoBR marked this conversation as resolved.
Expand Down
Loading