Skip to content

gitattributes: make install/kubernetes driver match more specific.#43943

Merged
joestringer merged 3 commits intocilium:mainfrom
tommyp1ckles:pr/thadlaw/improve-install-k8s-merge-drivers
Jan 27, 2026
Merged

gitattributes: make install/kubernetes driver match more specific.#43943
joestringer merged 3 commits intocilium:mainfrom
tommyp1ckles:pr/thadlaw/improve-install-k8s-merge-drivers

Conversation

@tommyp1ckles
Copy link
Copy Markdown
Contributor

Our current drivers are matched on everything inside install/kubernetes. When you do git merge ..., git will check the return value of any matching drivers.
In the case of the install/k8s dir we have the kubernetes-update driver. However, because this presumably completes and returns 0, this also papers over any legitimate conflicts.

For example, if there is a conflict in cilium/values.yaml that requires regenerating, but also a legitimate conflict in another (non-generated) file then this will silently run the kubernetes-update driver commands and silently skip incoming changes for other files.

This commit tries to make that matching more specific, by matching only on files we know get overwritten by code generation we avoid wrongly auto-merging other changes.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 23, 2026
@tommyp1ckles tommyp1ckles added the release-note/misc This PR makes changes that have no direct user impact. label Jan 23, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 23, 2026
@tommyp1ckles tommyp1ckles marked this pull request as ready for review January 23, 2026 01:14
@tommyp1ckles tommyp1ckles requested a review from a team as a code owner January 23, 2026 01:14
@tommyp1ckles tommyp1ckles requested a review from bimmlerd January 23, 2026 01:14
Copy link
Copy Markdown
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

LGTM. Nice catch, this is why auto conflict resolution is so scary 😞

Copy link
Copy Markdown
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this. A couple of comments inline.

@tommyp1ckles tommyp1ckles force-pushed the pr/thadlaw/improve-install-k8s-merge-drivers branch from b718582 to 0cc5deb Compare January 24, 2026 02:04
@giorio94 giorio94 added needs-backport/1.16 needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Jan 26, 2026
Copy link
Copy Markdown
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

Thanks! Changes look reasonable to me based on my limited knowledge of merge drivers functioning. Only one suggestion inline.

@tommyp1ckles
Copy link
Copy Markdown
Contributor Author

Thanks! Changes look reasonable to me based on my limited knowledge of merge drivers functioning.

I don't have much experience here either, but I'm hoping this fixes some of the obvious issues - we may need to revisit. It may also be hard to keep some of these path globs up to do indefinitely but let's see how this goes.

@tommyp1ckles tommyp1ckles force-pushed the pr/thadlaw/improve-install-k8s-merge-drivers branch from 0cc5deb to 2f0f97b Compare January 27, 2026 00:29
@tommyp1ckles
Copy link
Copy Markdown
Contributor Author

/test

Our current drivers are matched on everything inside install/kubernetes.
When you do `git merge ...`, git will check the return value of any
matching drivers.
In the case of the install/k8s dir we have the kubernetes-update driver.
However, because this presumably completes and returns 0, this also
papers over any legitimate conflicts.

For example, if there is a conflict in cilium/values.yaml that
requires regenerating, but also a legitimate conflict in another
(non-generated) file then this will silently run the kubernetes-update
driver commands and silently skip incoming changes for other files.

This commit tries to make that matching more specific, by matching
only on files we know get overwritten by code generation we avoid
wrongly auto-merging other changes.

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
go.mod is not generated, therefore the go mod resolution driver
won't correctly fix merge conflicts. This removes that line
and leaves the auto merge driver to just match the go.sum/vendor files.

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Similar to previous commits we avoid bad auto merges as
well as reducing redundant code generation runs by matching
individual api types with specific merge drivers that only
run make generate-x-api commands relevant to the directory.

Previously we would always run make-generate-apis multiple times
which alongside performing incorrect auto-merges also takes a long
time to complete.

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles tommyp1ckles force-pushed the pr/thadlaw/improve-install-k8s-merge-drivers branch from 2f0f97b to 9667438 Compare January 27, 2026 00:34
@tommyp1ckles
Copy link
Copy Markdown
Contributor Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 27, 2026
@joestringer joestringer enabled auto-merge January 27, 2026 21:36
@joestringer joestringer added this pull request to the merge queue Jan 27, 2026
Merged via the queue into cilium:main with commit 53f4474 Jan 27, 2026
76 checks passed
@mhofstetter mhofstetter mentioned this pull request Jan 29, 2026
6 tasks
@mhofstetter mhofstetter added backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. and removed needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Jan 29, 2026
@mhofstetter mhofstetter mentioned this pull request Jan 29, 2026
6 tasks
@mhofstetter mhofstetter added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Jan 29, 2026
@mhofstetter mhofstetter mentioned this pull request Jan 29, 2026
2 tasks
@mhofstetter mhofstetter added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jan 29, 2026
@mhofstetter mhofstetter mentioned this pull request Jan 29, 2026
1 task
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. backport-pending/1.19 The backport for Cilium 1.19.x for this PR is in progress. backport-pending/1.16 backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Jan 30, 2026
@cilium-release-bot cilium-release-bot bot moved this to Released in cilium v1.19.0 Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

6 participants