Skip to content

Conversation

@jotak
Copy link
Member

@jotak jotak commented Mar 7, 2022

  • an image tagged "main" (tracking branch "main")
  • an image, short-lived, tagged after commit SHA

@jotak
Copy link
Member Author

jotak commented Mar 7, 2022

This is to have similar images as the ones created on the other repos.
By the way, if you think it's useful to keep latest as well, we can add it back in addition to main

@eranra
Copy link
Collaborator

eranra commented Mar 8, 2022

@jotak I think that having a "latest" tag is useful and common so I suggest adding that back
@jotak can you explain more the purpose of this code ... the trigger to this action is on: push: branches: [ main ] --- so we get only main branch push

@jotak
Copy link
Member Author

jotak commented Mar 8, 2022

@jotak I think that having a "latest" tag is useful and common so I suggest adding that back

ok

@jotak can you explain more the purpose of this code ... the trigger to this action is on: push: branches: [ main ] --- so we get only main branch push

At the moment there's only a main branch, but at some point there will be more branches and we should then update the script to include them. This can be needed to backport fixes that target a version developed for a specific OCP version. We must consider the full picture operator+FLP+console plugin for that. There's sometimes breaking changes (e.g. in the pipeline config, like we've seen in the last few days) that makes it necessary to keep track of the version released for a given OCP version as a branch. If we adopt semver, it could be done with branches like v0.1.x, v0.2.x, and for a given OCP release, the netobserv components are bound to such a version branch.
Since we haven't released FLP yet, we still only have main, but we should have more soon.
To get back to image tagging, this build script will have to evolve, but the idea is to have one tag per branch in the future.

@jotak jotak force-pushed the sha-images branch 2 times, most recently from d4f2ae2 to 88a6080 Compare March 8, 2022 09:42
@jotak
Copy link
Member Author

jotak commented Mar 8, 2022

@eranra I did a few changes, adding latest and removing the temporary Dockerfile (replaced with shortlived.Dockerfile in contrib dir), hope it makes the Makefile more easy to reason about

jotak added 2 commits March 8, 2022 11:04
- an image tagged "main" (tracking branch "main")
- an image, short-lived, tagged after commit SHA
@jotak jotak requested a review from eranra March 8, 2022 10:17
It's the same as in console-pluigin repo
@jotak
Copy link
Member Author

jotak commented Mar 8, 2022

I've also added a release action, same as in the console plugin repo
I haven't fully tested it, will do when we can try a release candidate

@codecov-commenter
Copy link

Codecov Report

Merging #129 (18dc917) into main (93758ac) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #129   +/-   ##
=======================================
  Coverage   57.14%   57.14%           
=======================================
  Files          49       49           
  Lines        2842     2842           
=======================================
  Hits         1624     1624           
  Misses       1103     1103           
  Partials      115      115           
Flag Coverage Δ
unittests 57.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93758ac...18dc917. Read the comment docs.

@eranra
Copy link
Collaborator

eranra commented Mar 8, 2022

@eranra I did a few changes, adding latest and removing the temporary Dockerfile (replaced with shortlived.Dockerfile in contrib dir), hope it makes the Makefile more easy to reason about

Yes .. very good change :-)

- name: validate tag
id: validate_tag
run: |
tag=`git describe --exact-match --tags 2> /dev/null`
Copy link
Collaborator

Choose a reason for hiding this comment

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

A question and a suggestion ... we have some of that logic in the version building in the Makfile ... would it make sense to expose that with one a Makefile target instead of writing all this logic here ... maybe move to a script under hack ... I just think that having so much logic inside github action might not be good practice?

This is just a suggestion ... not blocking merge or something like that ,.,., this is good now :-0)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe one downside if we have it in a separate place, it's that the echo "::set-output name=tag::$tag" command, which is tied to the rest of the github action, would be less visible from the action, so it could be more confusing or error prone if the script is modified. What do you think?

@eranra
Copy link
Collaborator

eranra commented Mar 8, 2022

I've also added a release action, same as in the console plugin repo I haven't fully tested it, will do when we can try a release candidate

👍

@jotak jotak merged commit 4125851 into netobserv:main Mar 8, 2022
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