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

How to Overwrite Destination Branch and Keep One Single Commit #735

Open
squat opened this issue Aug 15, 2024 · 9 comments
Open

How to Overwrite Destination Branch and Keep One Single Commit #735

squat opened this issue Aug 15, 2024 · 9 comments

Comments

@squat
Copy link

squat commented Aug 15, 2024

My goal is that I want my ImageUpdateAutomation to push commits to a bump-x branch, always overriding any old commit in the branch and keeping a single commit that updates an image tag in the main branch directly to the newest image tag in my registry.

Image updates are working in my repo today, however when multiple image tags have been pushed since the last time the bump-x branch was merged to main, then my bump-x branch will have multiple commits in it. This is annoying because I use a commit template that inserts a link to the GitHub comparison between the the commit SHA found in the newest image tag and the commit SHA found in the second newest image tag. However, the comparison links that are generated are always for the difference between this newest image tag and the image tag found in the previous commit in the bump-x branch, not the difference between the newest image tag and the image tag currently on the main branch. My automation uses the body of the latest commit message as the PR body, so that reviewers can easily click on comparison links and know what changes are going into production, however these comparison links are misleading today because they only show a fraction of the whole changeset.

For this reason, I'd like to find way for the image-automation-controller to always plan the commit using the main branch and then force push that to the bump-x branch. Reading the documentation, I thought this would be possible using the refspec field:

In the following snippet, updates and commits will be made on the main branch locally. The commits will be then pushed using the refs/heads/main:refs/heads/auto refspec:

spec:
 git:
   checkout:
     ref:
       branch: main
   push:
     refspec: refs/heads/main:refs/heads/auto

From my reading, Flux would work on the main branch locally and then push images to the auto branch. In my case, I used a refspec of refs/heads/main:refs/heads/bump-x. However, this had a very unintended result where the changeset was pushed directly to the main branch on the remote.

How can I achieve the desired goal of always having a single commit on the bump-x branch? If using refspecs is the way to do it, then I'm confused about the semantics of the refspec field and how it interacts with branches that are explicitly or implicitly defined in an ImageUpdateAutomation.

@kingdonb
Copy link
Member

Hello @squat! I'm trying to follow the Gerrit / refspec docs, but I'm not sure I'm fully understanding. We are talking about it in Flux Bug Scrub now.

I see in the spec.git.push.refspec documentation, there is a link to "Gerrit" with more details about how to use this with Gerrit:

https://fluxcd.io/flux/components/image/imageupdateautomations/#gerrit

Have you seen this already? I'm still wrapping my head around these docs, but reading carefully I noticed two things differ from the example in the refspec - and maybe this is an error to correct in the documentation, not sure yet.

spec:
  git:
    checkout:
      ref:
        branch: main
    commit:
      author:
        email: flux@localdomain
        name: flux
      messageTemplate: |
        ...
    push:
      branch: auto
      refspec: refs/heads/auto:refs/heads/main

Note that checkout.ref.branch is main, but push.branch is set to auto - this differs from the example under refspec

Note also that refspec here reverses the order. This push.refspec has refs/heads/auto:refs/heads/main instead of the inverted refs/heads/main:refs/heads/auto - I'm not sure which of these is correct. I would guess based on how the Git CLI works, that the left hand side is the select-from branch from the local clone, and the right-hand side is the push-to branch or the remote branch.

The branch auto here in the example is your branch bump-x in your example.

We're sorry if the documentation is unclear! Have you seen this other Gerrit section, and does it help clarify the issue at all? I'd like to understand the Gerrit use case a bit better before I get any more specific, because I am not a Gerrit user and it's not exactly clear to me how a ChangeSet in Gerrit works.

Please have a look, and let us know if this is any help.

@squat
Copy link
Author

squat commented Aug 15, 2024

Hi @kingdonb , yes I saw the Gerrit docs though I mostly ignored it since Gerrit is kind of its own thing. Some details in there caught my eye and seems relevant, e.g. (emphasis mine):

Gerrit operates differently from a standard Git server. Rather than sending individual commits to a branch, all changes are bundled into a single commit.

Concerning the inverted order of the refspec components in the Gerrit example, the Flux docs say (emphasis mine):

This instructs the image-automation-controller to clone the repository using the main branch but execute its update logic and commit with the provided message template on the auto branch. Commits are then pushed to the auto branch, followed by pushing the HEAD of the auto branch to the HEAD of the remote main branch.

Pushing to main is expressly something I want to avoid entirely. I want my single commit to end up in bump-x, after which a GitHub action will automatically open a commit for human review before merging to main.

My assumption is that there's no bug and I'm simply failing to understand the docs, how Flux deals with refspecs, and how to push a single bundled commit to a desired branch without pushing to main.

I read through the source code a bit and it looks to me like the Flux image-automation-controller will always push to a branch even if you don't specify one under push and even if a refspec is specified. This was not obvious to me and definitely explains why commits ended up in the main branch: https://github.com/fluxcd/image-automation-controller/blob/main/internal/source/source.go#L277-L296.

I'm still stumped on how to push a single, bundled commit to a remote branch.

@kingdonb
Copy link
Member

I think you want to set the checkout branch and the push branch, and avoid setting refspec altogether. You can set the checkout branch to main and the push branch to bump-x. Also consider setting force to true, this will wipe away any stale commits that are in bump-x and reset the base to whatever the current checked out commit in main is, every time it runs.

@squat
Copy link
Author

squat commented Aug 15, 2024

I think you want to set the checkout branch and the push branch, and avoid setting refspec altogether.

This is the original, non-working state of my configuration. With these settings, Flux pushes a new commit onto the bump-x branch for every single new tag that is discovered.

Also consider setting force to true, this will wipe away any stale commits that are in bump-x and reset the base to whatever the current checked out commit in main is, every time it runs.

The documentation seems to imply that force is enabled by default and that it can "alternatively" be disabled:

The push branch will be created locally if it does not already exist, starting from the checkout branch. If the push branch already exists, it will be overwritten with the cloned version plus the changes made by the controller. Alternatively, force push can be disabled by starting the controller with flag --feature-gates=GitForcePushBranch=false, in which case the updates will be calculated on top of any commits already on the push branch.

Is this not the case? Do I simply have to enable a feature flag?

@darkowlzz
Copy link
Contributor

Hi, I gave this a try and your observations are the expected default behavior. Force push is enabled by default, but the reason for including the old commits from the push branch is the GitAllBranchReferences feature gate, see https://fluxcd.io/flux/components/image/options/#feature-gates. This was added because of this issue fluxcd/flux2#3384 to prevent committing same change over and over. This feature takes into consideration the previous commit and helps avoid new commits if there's no new change.
There's some commentary about it in the code if that helps, refer https://github.com/fluxcd/image-automation-controller/blob/v0.38.0/internal/source/git.go#L113-L119.

Disabling this flag removes the old commits from the push branch.
My ImageUpdateAutomation configuration just had:

...
  git:
    checkout:
      ref:
        branch: main
    push:
      branch: update
...

No refspec needed.

@squat
Copy link
Author

squat commented Aug 16, 2024

@darkowlzz thanks! That's really helpful. That means that if I disable the feature flag, my Git repo will see the branch overwritten with new commit every <interval> seconds, right?

@darkowlzz
Copy link
Contributor

That means that if I disable the feature flag, my Git repo will see the branch overwritten with new commit every <interval> seconds, right?

Yes, but please give it a try and see if it meets your expectation. We do have some reconciliation optimizations when the checkout branch and push branch are the same to determine if there is any change by checking the image policy or the checkout branch to prevent unnecessary image update processing when nothing has changed. If no explicit push branch is defined, the checkout and push branch will be the same, and new image update commits will only be made, if any, if there's a change in the checkout branch or the image policy, at the reconciliation interval.

@squat
Copy link
Author

squat commented Aug 19, 2024

Thanks @darkowlzz, I'll give this a shot now. It sounds like the optimizations you were describing won't apply to the case at hand where push branch and checkout branches are different, e.g.

...
  git:
    checkout:
      ref:
        branch: main
    push:
      branch: update
...

since the optimizations seem to depend on the checkout and push branches being the same.

@squat
Copy link
Author

squat commented Sep 5, 2024

I finally got around to testing this on our cluster and I'm sad to report that the workaround proposed by @darkowlzz doesn't meet my expectations. Indeed, the image automation controller will push a new commit every time reconciliation is triggered. This is annoying because it triggers our CI e2e tests every time a new commit is pushed.

I still think it would be a valuable feature to be able to consolidate changes into a single commit without pushing if there is no diff between the current state of the target branch and the commit generated by the controller.

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

No branches or pull requests

3 participants