bob-cd / bob

This is what CI/CD should've been.
https://bob-cd.github.io
MIT License
218 stars 16 forks source link

[runner] add :initialized and :started timestamp #122

Closed aldosolorzano closed 1 year ago

aldosolorzano commented 1 year ago

There problem

There is lack of visibility on the timestamp lifecycle of a pipeline, there is a desire to know when the status changed.

Related to the issue of Record the timestamp of pipeline status changes and after some discussion the following outcomes were agreed;

lispyclouds commented 1 year ago

Thanks! Would you like to add some tests for this?

aldosolorzano commented 1 year ago

Yes, I'm still finishing some things. I thought the PR was going to be created only in my fork hehe

aldosolorzano commented 1 year ago

@lispyclouds not sure why the build is failing but tests seem to be passing. Let me know if you think we should add more tests.

lispyclouds commented 1 year ago

Right, so the build is failing at the container image push step as the secrets aren't shared with pull request pipeline runs. Will push in a change to make the build and deploy steps separate and only run on main.

lispyclouds commented 1 year ago

If you rebase main, you should have the optional deploy now.

aldosolorzano commented 1 year ago

@lispyclouds :done I changed the tests and rebased the main branch

aldosolorzano commented 1 year ago

could you re-run tests please? seems like a flaky failure

lispyclouds commented 1 year ago

Looks good, just that I think :initialized should be assoc'ed after the image is pulled in. We are trying to see when the pipeline has everything to get started.

aldosolorzano commented 1 year ago

So it would be after this line?

Then do we want to have an initialized or ready status? The way I understood is that every status change should match with a timestamp, if the pulling of the image fails we wont have a timestamp related to the initializing status.

lispyclouds commented 1 year ago

yeah it should be after the image pull and let's add in an initialized state too.

aldosolorzano commented 1 year ago

So, this has make me think more, I believe that adding the -at for the attributes that hold a timestamp makes them more obvious about their purpose and what type of value they hold, as of :started could be open to other interpretations. Maybe adding the -at is not something we want or a breaking change, it's okay, we can stick with the current way of doing it, we only need to decide the timestamp name for the initializing and the initialized status.

status timestamp alternative status timestap
:initializing :initializing-at :initializing initiated-at
:initialized :initialized-at :ready ready-at
:running :started-at
:passed :completed-at
:failed :completed-at
:stopped :completed-at
lispyclouds commented 1 year ago

Makes sense. Lets keep the following:

and the statuses like you described.

lispyclouds commented 1 year ago

My longer term plan is to have https://opentelemetry.io/ based tracing too, this is working towards it.

aldosolorzano commented 1 year ago

@lispyclouds suddenly I cannot run tests any more, I'm getting this error. Have you gotten that problem? only happens with the runner tests

java 19.0.2 2023-01-17 Java(TM) SE Runtime Environment (build 19.0.2+7-44) Java HotSpot(TM) 64-Bit Server VM (build 19.0.2+7-44, mixed mode, sharing)

Error: no container with name or ID "bob-test-storage" found: no such container
Error: no container with name or ID "bob-test-storage" found: no such container
Error: unable to find network configuration for bob-net: network not found
/home/aldo.solorzano/.config/cni/net.d/bob-net.conflist
Error: error creating container storage: the container name "podman" is already in use by "f92ccb5c6b73045a264c8e5e13b51e9145415745164f7058fe1a4cd8c1f4af80". You have to remove that container to be able to reuse that name.: that name is already in use
Error: no container with name or ID "bob-test-storage" found: no such container
Error: no container with name or ID "bob-test-storage" found: no such container
bob-net
----- Error --------------------------------------------------------------------
Type:     clojure.lang.ExceptionInfo
Message:  
Data:     {:proc #object[java.lang.ProcessImpl 0x69f4a814 "Process[pid=6658, exitValue=125]"], :exit 125, :in #object[java.lang.ProcessBuilder$NullOutputStream 0x4fcd4abf "java.lang.ProcessBuilder$NullOutputStream@4fcd4abf"], :out #object[java.lang.ProcessBuilder$NullInputStream 0x588b9940 "java.lang.ProcessBuilder$NullInputStream@588b9940"], :err #object[java.lang.ProcessBuilder$NullInputStream 0x588b9940 "java.lang.ProcessBuilder$NullInputStream@588b9940"], :prev nil, :cmd ["/usr/bin/podman" "run" "-d" "--name" "podman" "--device" "/dev/fuse" "--security-opt" "seccomp=unconfined" "--security-opt" "apparmor=unconfined" "--security-opt" "label=disable" "--cap-add" "sys_admin" "--cap-add" "mknod" "-p" "8080:8080" "docker.io/mgoltzsche/podman:minimal" "podman" "system" "service" "-t" "0" "tcp://0.0.0.0:8080"], :type :babashka.process/error}
Location: /home/aldo.solorzano/dev/experiments/bob/tasks/tasks.clj:52:4

----- Context ------------------------------------------------------------------
48: (defn crun
49:   ([cmd]
50:    (crun cmd {}))
51:   ([cmd opts]
52:    (process/shell (merge {:out :inherit :in :inherit :err :inherit} opts) (str (container-engine) " " cmd))))
       ^--- 
53: 
54: (defn scrun
55:   [cmd]
56:   (crun cmd {:continue true}))

----- Stack trace --------------------------------------------------------------
babashka.process/check                    - <built-in>
babashka.process/shell                    - <built-in>
tasks/crun                                - /home/aldo.solorzano/dev/experiments/bob/tasks/tasks.clj:52:4
tasks/crun                                - /home/aldo.solorzano/dev/experiments/bob/tasks/tasks.clj:50:4
tasks/crun                                - /home/aldo.solorzano/dev/experiments/bob/tasks/tasks.clj:48:1
user-c9e7036c-44c9-4d4d-ab36-958ed1d38ce1 - <expr>:26:55
aldosolorzano commented 1 year ago

I ran podman rm -f f92ccb5c6b73045a264c8e5e13b51e9145415745164f7058fe1a4cd8c1f4af80 and it worked

aldosolorzano commented 1 year ago

sorry for all the noise, it helps me debug things when I write them down somewhere. I think the PR is ready now

lispyclouds commented 1 year ago

Looks great! Thanks a lot for all the effort and now that you know the code, if you have some refactoring ideas/feedback, most welcome!

aldosolorzano commented 1 year ago

Thank you for all the support, It really made my first open source contribution awesome.

lispyclouds commented 1 year ago

ah! a warm welcome! hoping for more such experiences! ❤️