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

feat: bumped the version of the Pact Broker to 2.112.0-pactbroker2.107.1 (using the new docker tagging naming convention) #64

Merged

Conversation

sherifkayad
Copy link
Contributor

feat: bumped the version of the Pact Broker to 2.112.0-pactbroker2.107.1 (using the new docker tagging naming convention)

…7.1 (using the new docker tagging naming convention)

Signed-off-by: Sherif Ayad <sherif.k.ayad@gmail.com>
@sherifkayad
Copy link
Contributor Author

@ChrisJBurns would you mind having a look at this one? .. I changed the used image tag to match the new standard set in the docker project

My suggestion would be (to keep SemVersioning):

  • Any version change on the left handside (not application specific) is treated by this Chart as a patch
  • Any version change on the right is classfied into:
    • Major => results in a major version upgrade of the Chart
    • Minor => results in a minor version upgrade of the Chart
    • Patch => results in a patch version upgrade of the Chart

@ChrisJBurns
Copy link
Contributor

ChrisJBurns commented Sep 15, 2023

Hi @sherifkayad thanks for the PR, when you say right hand side, what do you mean exactly?

Chart versions tend to be loosely coupled (some not related at all) with the underlying application versions. I typically only tend to bump the major Chart versions if I'm introducing a change that will break existing deployments for users. I've not really needed to bump the application versions of the Pact Broker in this Chart for a while because it hasn't required any new resources or had issues with any current resources being created by the Chart. So in order for users to use the latest Pact Broker version they could just set the latest image as a value at install time (we do this in my company with tools like FluxCD and Renovate). Hope this makes sense, I'd still like to merge this PR because although the new Pact Broker image doesn't really require any Chart resource changes, it's good to at least align with the new tag scheme of the image 👍

@sherifkayad
Copy link
Contributor Author

sherifkayad commented Sep 16, 2023

@ChrisJBurns thanks for triggering the pipelines. What I meant by left / right hand side was the new naming scheme of the Pact Broker Docker Image and that images would have a version of the Base Docker Image (left hand side part) and concatenated to it the Pact Broker version (right hand side part).

Since a couple of projects now, we only maintain the Chart version and try not to maintain an extra Image Version in each chart (basically we don't override the default Image nor its Version) .. As all of the public Helm Charts bump their Chart version once the application Image is updated. That could be a simpler approach even if one has a Renovate Bot performing the upgrades..

I know that's probably different from the approach you mentioned, however, I believe it's way simpler for users of the Helm Chart.

Maybe a suggestion to make the maintenance of this Helm Chart easier is to connect it to Renovate and configure a semantic helm release Plugin to handle auto bumping the Chart version once the application Image changes .. we kinda do that approach for our internal Helm Charts.

@ChrisJBurns
Copy link
Contributor

@sherifkayad Do you have some code samples of the config needed for renovate? We have Dependabot activated on the repo at the moment, but I've not used it to do what you mentioned? Also, I think this is probably worth a separate issue in itself with the respective PR to add it in. If we can automate the Chart updates with the image updates then that's great.

Do you mind creating an issue describing the fact that we need to automate the Pact Broker image versions updates as well as the relevant Chart updates also?

@ChrisJBurns ChrisJBurns merged commit 7cf08ef into pact-foundation:master Sep 16, 2023
3 checks passed
@sherifkayad
Copy link
Contributor Author

sherifkayad commented Sep 18, 2023

@ChrisJBurns thanks for merging this PR .. To the Renovate / Dependabot topic:

  • I agree this needs to be a separate issue by itself (until then the best bet is to do manual patches / manual PRs to this repo)
  • Unfortunately I don't have a config handy for this image explicitly, however, what I can tell you is:

For that I created #67

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.

None yet

2 participants