-
Notifications
You must be signed in to change notification settings - Fork 213
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
Fix kie-issues #1217 - Replace quay.io with docker.io #3525
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,4 +17,4 @@ | |
# under the License. | ||
# | ||
|
||
quarkus.kogito.dev-services-trusty.image-name=quay.io/kiegroup/kogito-trusty-postgresql:${version.org.kie.kogito.trusty-service-postgresql} | ||
quarkus.kogito.dev-services-trusty.image-name=docker.io/apache/incubator-kie-kogito-trusty-postgresql:${version.org.kie.kogito.trusty-service-postgresql} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where this image is published? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not. We've deprecated it. Hence, we can either keep this image here for ubiquity, keep the outdated, or remove this addon for good. I'd rather take this decision for the maintainers of this addon. @gitgabrio do you know? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. k, thank you for the clarifications. I would like to preserve as much as possible the tracing decision, but it has to adjust to remove the image reference that has been removed. @gitgabrio would be possible to adjust the code to avoid use the image? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this property is for the devservice. We should remove the devservice or review it. I recommend doing so in another PR. Waiting for @gitgabrio inputs. :D |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,6 @@ mp.messaging.outgoing.kogito-tracing-model.topic=kogito-tracing-model | |
mp.messaging.outgoing.kogito-tracing-model.value.serializer=org.apache.kafka.common.serialization.StringSerializer | ||
mp.messaging.outgoing.kogito-tracing-model.auto.offset.reset=earliest | ||
|
||
quarkus.kogito.dev-services-trusty.image-name=quay.io/kiegroup/kogito-trusty-postgresql:latest | ||
quarkus.kogito.dev-services-trusty.image-name=docker.io/apache/incubator-kie-kogito-trusty-postgresql:latest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't find this image source; mind help locate it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same. |
||
#quarkus.kogito.dev-services-trusty.image-name=org.kie.kogito/trusty-service-postgresql:2.0.0-SNAPSHOT | ||
quarkus.kogito.dev-services-trusty.port-used-by-test=-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs adjustment.. For the main branch, the tag would be
main
, that's fine. But when the branch is notmain
, we actually agreed to use the branch name to represent the stream, so the tag of this image would be 10.0.x. Now when we're doing an ACTUAL release, meaning versions does not end with-SNAPSHOT
, then that's when we're supposed to usegetMajorMinor()
.. Let me know if I'm missing something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With that said, this is not something that should only be done on the deploy job, as the code itself needs to reflect that, so people building locally and developing will have the same behavior. I guess the best solution would be calling:
mvn versions:set-property -Dproperty=data-index-ephemeral.image -DnewVersion=<new-value>
on this module during commits D and R, as per our release procedure described here: https://docs.google.com/document/d/1t-08b970i9PMBy-TSM64x0RUH46pzwnIhEi-LVeL0lg/editThis way, the deployment CI doesn't need to alter this property when publishing, and the build from source is always accurate.
Better yet would be creating a new property to hold only have the
tagVersion
, not the fulltag
of the image. This new property (E.g.,data-index-ephemeral.image.tagVersion
) can then be used to compose the existingdata-index-ephemeral.image
property, likedocker.io/apache/incubator-kie-kogito-data-index-ephemeral:${data-index-ephemeral.image.tagVersion}
.Sorry for dumping the entire reasoning here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiagobento I think we need another PR targeting CI changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.