Implement CI using CircleCI #421

vprashar2929 · 2023-03-09T08:15:00Z

This PR is in continuation to this. It contains implementing RHOBS configuration CI using CircleCI and addresses some of the drawbacks of using GitHub Actions.
Currently the CI pipeline performs following actions:

Run microshift on Linux VM that is provided by CircleCI. Since RHOBS configs are specific to OCP so it's best to use OCP env to deploy and test these instead of kind/minikube.
Deploy's the generated manifests on microshift cluster with minimum resources.
Run a small test using observatorium/up (Like in observatorium e2e.sh) to verify send/receive metrics.

Using circleci we are getting good resources out of the box than existing GitHub Actions runners.

douglascamata · 2023-03-09T10:40:06Z

hey @vprashar2929, can you mention the drawbacks of Github Actions that makes us fallback to Circle CI, please, as we spoke some time ago? It's valuable to have the reasoning of the decision for documentation purposes.

simonpasquier · 2023-03-10T08:07:41Z

I'm really an outsider here but I agree with @douglascamata that more context would be helpful
I'd keep also in mind that CircleCI might reduce the free plan's resources at any time without prior notice. Do we have a backup plan for this possibility?

vprashar2929 · 2023-03-10T09:32:42Z

hey @vprashar2929, can you mention the drawbacks of Github Actions that makes us fallback to Circle CI, please, as we spoke some time ago? It's valuable to have the reasoning of the decision for documentation purposes.

So the only difference between the two according to our CI use case is in terms of out of the box (free of cost) resources which both has to offer. GitHub Actions provide 2 vCPU and 7GB or ram which are not sufficient enough to deploy complete RHOBS components in one go. Whereas CircleCI provides 4 vCPU and 15GB of ram.
Since we want to deploy RHOBS with minimum resources and run a simple test on generated manifests than currently it is easy to achieve using CircleCI instead of GitHub Actions.

vprashar2929 · 2023-03-10T09:38:29Z

I'm really an outsider here but I agree with @douglascamata that more context would be helpful I'd keep also in mind that CircleCI might reduce the free plan's resources at any time without prior notice. Do we have a backup plan for this possibility?

Oh reduce free plan resources hurts 😅. Well in that case what we can do is deploy each components individually and check its rollout status. But in this case we won't be able to deploy all components at once and run tests on it. It can only validate the deployments generated manifests.

douglascamata · 2023-03-10T09:52:04Z

@simonpasquier @vprashar2929 my proposals for the case where CircleCI reduces the size of the free runner:

We join the paid plan (lol) of whichever of the two vendors has the best cost/benefit ratio for our case.
We run a private Github Action Runner for the builds of the RHOBS org.

I personally don't worry too much in case we need a backup plan. 👍

vprashar2929 · 2023-03-10T09:58:17Z

@douglascamata Absolutely agree with you. We can definitely do a cost analysis between the two and choose what best suits us according to cost. Provided we are willing to pay the bucks😄
Also I am open to self-hosted GitHub runners specifically for our RHOBS repo as in that case we can have a dedicated OCP cluster running and escape from resources crunch completely.

douglascamata · 2023-03-16T11:20:27Z

tests/ci/ci_test.sh

@vprashar2929 was wondering if we could better separate here the deploy procedures and the test ones, so that we can reuse the test part in a post-deploy job. What do you think? This is not a blocker though. I am happy with merging this as is now and then follow up with the separation and more tests.

CI test procedures consist of just checking the rollout status of various deployments/statefulsets along with running observatorium-up to send/receive metrics.

The current post deploy job runs inside a docker with observatorium-up binary copied to Alpine-based container and a runtest.sh script is triggered inside the container. Since this script was already created specifically for post deploy job I think we can reuse this script for adding further checks to job. What are your thoughts?

Sounds good to me. 👍

douglascamata

LGTM.

Would be great to have a review from other team members (cc @rhobs/team-monitoring)

sthaha · 2023-03-19T06:25:22Z

tests/ci/ci_test.sh

@@ -0,0 +1,140 @@
+#!/bin/bash
+
+set -e


I think it s best to have -u set for all bash script. I good template to follow in imho is

#!/usr/bin/env bash set -eu -opipefail main() { return 0 } main "$@"

Definitely will add -u flag. Will avoid any undefined behaviour in script

sthaha · 2023-03-19T06:26:07Z

tests/ci/ci_test.sh

+set -e
+set -o pipefail
+
+ARTIFACT_DIR="${ARTIFACT_DIR:-/tmp/artifacts}"


declare -r for all readonly vars

sthaha · 2023-03-19T06:27:12Z

tests/ci/ci_test.sh

+set -o pipefail
+
+ARTIFACT_DIR="${ARTIFACT_DIR:-/tmp/artifacts}"
+INFO="INFO"


why is this required? where do we change this variable?

Earlier I used two variables for Logging purposes INFO and ERROR. But since we are gathering Artifacts in case of any failure I think its best to remove this

sthaha · 2023-03-19T06:29:26Z

tests/ci/ci_test.sh

+    res=$1
+    namespace=$2
+    log_info "Checking status of $res inside $namespace"
+    oc rollout status $res -n $namespace --timeout=5m || (must_gather "$ARTIFACT_DIR" && exit 1)


Do we need a subshell here?

Suggested change

oc rollout status $res -n $namespace --timeout=5m || (must_gather "$ARTIFACT_DIR" && exit 1)

oc rollout status $res -n $namespace --timeout=5m || {

must_gather "$ARTIFACT_DIR"

return 1

}

Not mandatory. We can use {} along with || as well as it will also allow executing multiple commands if the oc rollout status fails

sthaha · 2023-03-19T06:29:51Z

tests/ci/ci_test.sh

+}
+minio(){


nit: a blank line between functions will help with readability.

sthaha · 2023-03-19T06:30:22Z

tests/ci/ci_test.sh

+}
+prereq(){
+    log_info "Deploying prerequisites on cluster"
+    oc apply -f pre-requisites.yaml


should we use apply ? or create ?

sthaha · 2023-03-19T06:31:36Z

tests/ci/ci_test.sh

+minio(){
+    log_info "Deploying resources inside minio namespace"
+    oc create ns minio || true
+    sleep 5


sleep is usually a code smell. It is better to "poll" or wait for something to happen.

Sure will add oc wait to wait for namespace to be created.

sthaha · 2023-03-19T06:32:37Z

tests/ci/ci_test.sh

+    log_info "Deploying resources inside minio namespace"
+    oc create ns minio || true
+    sleep 5
+    oc process -f ../minio-template.yaml -p MINIO_CPU_REQUEST=30m -p MINIO_CPU_LIMITS=50m -p MINIO_MEMORY_REQUEST=50Mi -p MINIO_MEMORY_LIMITS=100Mi --local -o yaml | sed -e 's/storage: [0-9].Gi/storage: 0.25Gi/g' | oc apply -n minio -f -


Suggested change

oc process -f ../minio-template.yaml -p MINIO_CPU_REQUEST=30m -p MINIO_CPU_LIMITS=50m -p MINIO_MEMORY_REQUEST=50Mi -p MINIO_MEMORY_LIMITS=100Mi --local -o yaml | sed -e 's/storage: [0-9].Gi/storage: 0.25Gi/g' | oc apply -n minio -f -

oc process -f ../minio-template.yaml \

-p MINIO_CPU_REQUEST=30m \

-p MINIO_CPU_LIMITS=50m \

-p MINIO_MEMORY_REQUEST=50Mi \

-p MINIO_MEMORY_LIMITS=100Mi \

--local -o yaml |

sed -e 's/storage: [0-9].Gi/storage: 0.25Gi/g' |

oc apply -n minio -f -

VIM effects😅

sthaha · 2023-03-19T06:34:57Z

tests/ci/ci_test.sh

+case $1 in
+metrics)
+    prereq
+    minio
+    dex
+    observatorium_metrics
+    observatorium
+    run_test
+    telemeter
+    ;;
+logs)
+    #TODO
+    ;;
+traces)
+    #TODO
+    ;;
+*)
+    echo "usage: $(basename "$0") { metrics | logs | traces }"
+    ;;
+esac


Suggested change

case $1 in

metrics)

prereq

minio

dex

observatorium_metrics

observatorium

run_test

telemeter

;;

logs)

#TODO

;;

traces)

#TODO

;;

*)

echo "usage: $(basename "$0") { metrics | logs | traces }"

;;

esac

main() {

case $1 in

metrics)

prereq

minio

dex

observatorium_metrics

observatorium

run_test

telemeter

;;

logs)

#TODO

;;

traces)

#TODO

;;

*)

echo "usage: $(basename "$0") { metrics | logs | traces }"

return 1

;;

esac

}

main "$@"

Also just FYI instead of case, you can directly jump to a function like so

ci.metrics() { : } ci.logs() { : } ci.help() { local exit_code=${1:-0} local fns=($(declare -F -p | cut -f3 -d ' ' | grep '^et\.' | cut -f2- -d.)) read -d '^' -r docstring <<EOF_HELP Usage: $(basename "$0") <task> Tasks: $(printf " - %s\n" "${fns[@]}") ^ after this won't be read EOF_HELP echo -e "$docstring" exit "$exit_code" } is_function() { local fn=$1 shift [[ $(type -t "$fn") == "function" ]] return $? } main() { local fn=${1:-''} local ci_fn="ci.$fn" if ! is_function "$ci_fn"; then ci.help 1 fi # call the function passing in the reset of the args shift $ci_fn "$@" return $? }

Awesome @sthaha 🙇🏼‍♂️. I haven't tried like this but this looks good. Will definitely use this!

douglascamata reviewed Mar 16, 2023

View reviewed changes

douglascamata approved these changes Mar 17, 2023

View reviewed changes

sthaha reviewed Mar 19, 2023

View reviewed changes

vprashar2929 force-pushed the circle-ci branch from a7b914b to 1c2ab84 Compare March 23, 2023 10:16

vprashar2929 requested review from sthaha and periklis March 24, 2023 05:01

vprashar2929 and others added 10 commits April 5, 2023 18:27

Implement CI using Circleci

ffb009b

Modify config.yaml and add missing new line

f9c149f

Add build require in tests workflow

03a1f40

Update crds in ci_test.sh

c456e97

Fix path of crd template

b34d6e7

Update env for disabling log rules in obsctl reloader

b1abc5b

Update prometheus-operator crd link

43ed88f

Incorporate review comments

fa2dc2a

Add check for restartCount in run_test

6da09bd

Update go version

9471574

vprashar2929 force-pushed the circle-ci branch from ad8b820 to 9471574 Compare April 5, 2023 12:58

philipgough merged commit 0b24e59 into rhobs:main Apr 5, 2023

douglascamata mentioned this pull request Apr 5, 2023

Implement CI for RHOBS #418

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CI using CircleCI #421

Implement CI using CircleCI #421

vprashar2929 commented Mar 9, 2023 •

edited

Loading

douglascamata commented Mar 9, 2023 •

edited

Loading

simonpasquier commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

douglascamata commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

douglascamata Mar 16, 2023

vprashar2929 Mar 16, 2023

douglascamata Mar 17, 2023

douglascamata left a comment

sthaha Mar 19, 2023 •

edited

Loading

vprashar2929 Mar 20, 2023

sthaha Mar 19, 2023

sthaha Mar 19, 2023

vprashar2929 Mar 20, 2023

sthaha Mar 19, 2023

vprashar2929 Mar 20, 2023

sthaha Mar 19, 2023

sthaha Mar 19, 2023

sthaha Mar 19, 2023

vprashar2929 Mar 20, 2023

sthaha Mar 19, 2023

vprashar2929 Mar 20, 2023

sthaha Mar 19, 2023

sthaha Mar 19, 2023 •

edited

Loading

vprashar2929 Mar 20, 2023

-    oc process -f ../minio-template.yaml -p MINIO_CPU_REQUEST=30m -p MINIO_CPU_LIMITS=50m -p MINIO_MEMORY_REQUEST=50Mi -p MINIO_MEMORY_LIMITS=100Mi --local -o yaml | sed -e 's/storage: [0-9].Gi/storage: 0.25Gi/g' | oc apply -n minio -f -
+    oc process -f ../minio-template.yaml \
+      -p MINIO_CPU_REQUEST=30m \
+      -p MINIO_CPU_LIMITS=50m \
+      -p MINIO_MEMORY_REQUEST=50Mi \
+      -p MINIO_MEMORY_LIMITS=100Mi \
+      --local -o yaml |
+    sed -e 's/storage: [0-9].Gi/storage: 0.25Gi/g' |
+    oc apply -n minio -f -

		}
		minio(){

Implement CI using CircleCI #421

Implement CI using CircleCI #421

Conversation

vprashar2929 commented Mar 9, 2023 • edited Loading

douglascamata commented Mar 9, 2023 • edited Loading

simonpasquier commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

douglascamata commented Mar 10, 2023

vprashar2929 commented Mar 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

douglascamata left a comment

Choose a reason for hiding this comment

sthaha Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sthaha Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vprashar2929 commented Mar 9, 2023 •

edited

Loading

douglascamata commented Mar 9, 2023 •

edited

Loading

sthaha Mar 19, 2023 •

edited

Loading

sthaha Mar 19, 2023 •

edited

Loading