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

Draft PR for review comments of infraclouds contributions #2

Draft
wants to merge 449 commits into
base: infracloud/cassandra-5.0
Choose a base branch
from

Conversation

michaelsembwever
Copy link
Member

No description provided.

kubectl cp -n $KUBE_NS $POD_NAME:$BUILD_DIR/$build_number/log $LOCAL_DIR/$build_number/log
kubectl cp -n $KUBE_NS $POD_NAME:$BUILD_DIR/$build_number/junitResult.xml $LOCAL_DIR/$build_number/junitResult.xml


Copy link
Member Author

@michaelsembwever michaelsembwever Oct 16, 2023

Choose a reason for hiding this comment

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

we're missing here the teardown of the jenkins stack.
and this needs to be optional via a parameter.

something along the lines of

# teardown # TODO add option to skip this
kubectl delete --namespace ${KUBE_NS} -f ${CASSANDRA_DIR}/build/jenkins-deployment.yaml
kubectl delete "$(kubectl api-resources --namespaced=true --verbs=delete -o name | tr "\n" "," | sed -e 's/,$//')" --all
[ "default" == "${KUBE_NS}" ] ||  kubectl delete namespace ${KUBE_NS}

Copy link
Member Author

Choose a reason for hiding this comment

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

you can leave this, i will tackle it.

Choose a reason for hiding this comment

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

Since for now, the jenkins deployment is working with default namespace only (the limitation we mentioned earlier), so there is no need to delete the default namespace, we can remove the delete command (for namespace)

Copy link

Choose a reason for hiding this comment

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

We have added the teardown code, also parameterised it as well

.build/run-ci.sh Outdated
Comment on lines 89 to 92
sed -i -e "/targets:/s|:.*$|: \"$TARGETS\"|" \
-e "/repositoryBranch:/s|:.*$|: \"$REPO_BRANCH\"|" \
-e "/repositoryUrl:/s|:.*$|: \"$REPO_URL\"|" jenkins-deployment.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

fix needed for macos compatibility

Suggested change
sed -i -e "/targets:/s|:.*$|: \"$TARGETS\"|" \
-e "/repositoryBranch:/s|:.*$|: \"$REPO_BRANCH\"|" \
-e "/repositoryUrl:/s|:.*$|: \"$REPO_URL\"|" jenkins-deployment.yaml
sed -e "/targets:/s|:.*$|: \"$TARGETS\"|" \
-e "/repositoryBranch:/s|:.*$|: \"$REPO_BRANCH\"|" \
-e "/repositoryUrl:/s|:.*$|: \"$REPO_URL\"|" ${CASSANDRA_DIR}/.build/jenkins-deployment.yaml > ${CASSANDRA_DIR}/build/jenkins-deployment.yaml

.build/run-ci.sh Outdated
echo "Jenkins Operator installed successfully!" # condition to check if above command was success

# deploy jenkins Instance TODO jenkins file parameter
kubectl apply --namespace ${KUBE_NS} -f ${CASSANDRA_DIR}/.build/jenkins-deployment.yaml
Copy link
Member Author

Choose a reason for hiding this comment

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

to go with the fix above

Suggested change
kubectl apply --namespace ${KUBE_NS} -f ${CASSANDRA_DIR}/.build/jenkins-deployment.yaml
kubectl apply --namespace ${KUBE_NS} -f ${CASSANDRA_DIR}/build/jenkins-deployment.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

can we delete this file now ?

Copy link

Choose a reason for hiding this comment

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

This file has a code block (artifacts stage), which will require modifications of the existing bash scripts but it will help with getting rid of DinD. So, keeping this file for reference purposes. Also renaming it to k8s_POC.jenkins

.build/run-ci.sh Outdated Show resolved Hide resolved
.build/run-ci.sh Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

3 participants