[SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #2969

skanakal · 2024-10-21T11:02:58Z

Is there an existing issue for this?

I have searched the existing issues

Current Behavior

When disablePolling is enabled in a Git repository, Fleet fetches the latest commit but does not apply it to the deployment. The same behavior is observed with Azure DevOps.

Expected Behavior

when disablePolling set in gitrepo, New commits from webhook should trigger deployments

Steps To Reproduce

Install rancher 2.9.2 / fleet v0.10.3 (enable debug logging on fleet)
configure the Fleet webhook
create a girepo using example with simple path and set disablePolling: true
update the deployment (ex: change the replicas count to a higher number)
Notice the Fleet fetches the new commit in the gitrepo...
gitjob pod logs shows a webhook payload.

2024-10-21T09:46:20Z	DEBUG	webhook	Webhook payload	{"payload": {"ref":"refs/heads/main","before":"bf810dec76b610ba01271b6899306b63189c6eba","after":"afa5f4f644233e16f65ee3251d0522358d72fc02","created":false,"deleted":false,"forced":false,"base_ref":null,"compare":"https://github.com/xxx/test/compare/bf810dec76b6...afa5f4f64423","commits":[{"sha":"","id":"afa5f4f644233e16f65ee3251d0522358d72fc02","node_id":"","tree_id":"7bbe0c768f77fda465980cc5e2597bd50615f11b","distinct":true,"message":"Update deployment.yaml","timestamp":"2024-10-21T15:16:18+05:30","url":"https://github.

until I force update the gitrepo, there is no job created with the latest commit

Environment

- Architecture: x86_64
- Fleet Version: 0.10.3
- Cluster: local
  - Provider: custom
  - Options:
  - Kubernetes Version: 1.28

Logs

2024-10-21T10: 50: 26Z	DEBUG	webhook	Webhook payload	{
    "payload": {
        "ref": "refs/heads/main",
        "before": "afa5f4f644233e16f65ee3251d0522358d72fc02",
        "after": "797aa0b191af4b6f386eb8ccefc929c9baf8f35b",
        "created": false,
        "deleted": false,
        "forced": false,
        "base_ref": null,
        "compare": "https://github.com/xxxxxx/test/compare/afa5f4f64423...797aa0b191af",
        "commits": [
            {
                "sha": "",
                "id": "797aa0b191af4b6f386eb8ccefc929c9baf8f35b",
                "node_id": "",
                "tree_id": "28939fa43e9fa0c1724ecd392aca5d0d599d690e",
                "distinct": true,
                "message": "Update deployment.yaml",
                "timestamp": "2024-10-21T16:20:24+05:30",
                "url": "https://github.com/xxxxxx/test/commit/797aa0b191af4b6f386eb8ccefc929c9baf8f35b",
                "author": {
                    "name": "xxxxxx",
                    "email": "[email protected]",
                    "username": "xxxxxx"
                },
                "committer": {
                    "name": "GitHub",
                    "email": "[email protected]",
                    "username": "web-flow"
                },
                "added": [],
                "removed": [],
                "modified": [
                    "helloapp/deployment.yaml"
                ]
            }
        ],
        "head_commit": {
            "id": "797aa0b191af4b6f386eb8ccefc929c9baf8f35b",
            "node_id": "",
            "tree_id": "28939fa43e9fa0c1724ecd392aca5d0d599d690e",
            "distinct": true,
            "message": "Update deployment.yaml",
            "timestamp": "2024-10-21T16:20:24+05:30",
            "url": "https://github.com/xxxxxx/test/commit/797aa0b191af4b6f386eb8ccefc929c9baf8f35b",
            "author": {
                "name": "xxxxxx",
                "email": "[email protected]",
                "username": "xxxxxx"
            },
            "committer": {
                "name": "GitHub",
                "email": "[email protected]",
                "username": "web-flow"
            },
            "added": [],
            "removed": [],
            "modified": [
                "helloapp/deployment.yaml"
            ]
        },
        "repository": {
            "id": 873356537,
            "node_id": "R_kgDONA5c-Q",
            "name": "test",
            "full_name": "xxxxxx/test",
            "owner": {
                "login": "xxxxxx",
                "id": 53037653,
                "node_id": "MDQ6VXNlcjUzMDM3NjUz",
                "avatar_url": "https://avatars.githubusercontent.com/u/53037653?v=4",
                "gravatar_id": "",
                "url": "https://api.github.com/users/xxxxxx",
                "html_url": "https://github.com/xxxxxx",
                "followers_url": "https://api.github.com/users/xxxxxx/followers",
                "following_url": "https://api.github.com/users/xxxxxx/following{/other_user}",
                "gists_url": "https://api.github.com/users/xxxxxx/gists{/gist_id}",
                "starred_url": "https://api.github.com/users/xxxxxx/starred{/owner}{/repo}",
                "subscriptions_url": "https://api.github.com/users/xxxxxx/subscriptions",
                "organizations_url": "https://api.github.com/users/xxxxxx/orgs",
                "repos_url": "https://api.github.com/users/xxxxxx/repos",
                "events_url": "https://api.github.com/users/xxxxxx/events{/privacy}",
                "received_events_url": "https://api.github.com/users/xxxxxx/received_events",
                "type": "User",
                "site_admin": false
            },
            "private": false,
            "html_url": "https://github.com/xxxxxx/test",
            "description": "",
            "fork": false,
            "url": "https://github.com/xxxxxx/test",
            "forks_url": "https://api.github.com/repos/xxxxxx/test/forks",
            "keys_url": "https://api.github.com/repos/xxxxxx/test/keys{/key_id}",
            "collaborators_url": "https://api.github.com/repos/xxxxxx/test/collaborators{/collaborator}",
            "teams_url": "https://api.github.com/repos/xxxxxx/test/teams",
            "hooks_url": "https://api.github.com/repos/xxxxxx/test/hooks",
            "issue_events_url": "https://api.github.com/repos/xxxxxx/test/issues/events{/number}",
            "events_url": "https://api.github.com/repos/xxxxxx/test/events",
            "assignees_url": "https://api.github.com/repos/xxxxxx/test/assignees{/user}",
            "branches_url": "https://api.github.com/repos/xxxxxx/test/branches{/branch}",
            "tags_url": "https://api.github.com/repos/xxxxxx/test/tags",
            "blobs_url": "https://api.github.com/repos/xxxxxx/test/git/blobs{/sha}",
            "git_tags_url": "https://api.github.com/repos/xxxxxx/test/git/tags{/sha}",
            "git_refs_url": "https://api.github.com/repos/xxxxxx/test/git/refs{/sha}",
            "trees_url": "https://api.github.com/repos/xxxxxx/test/git/trees{/sha}",
            "statuses_url": "https://api.github.com/repos/xxxxxx/test/statuses/{sha}",
            "languages_url": "https://api.github.com/repos/xxxxxx/test/languages",
            "stargazers_url": "https://api.github.com/repos/xxxxxx/test/stargazers",
            "contributors_url": "https://api.github.com/repos/xxxxxx/test/contributors",
            "subscribers_url": "https://api.github.com/repos/xxxxxx/test/subscribers",
            "subscription_url": "https://api.github.com/repos/xxxxxx/test/subscription",
            "commits_url": "https://api.github.com/repos/xxxxxx/test/commits{/sha}",
            "git_commits_url": "https://api.github.com/repos/xxxxxx/test/git/commits{/sha}",
            "comments_url": "https://api.github.com/repos/xxxxxx/test/comments{/number}",
            "issue_comment_url": "https://api.github.com/repos/xxxxxx/test/issues/comments{/number}",
            "contents_url": "https://api.github.com/repos/xxxxxx/test/contents/{+path}",
            "compare_url": "https://api.github.com/repos/xxxxxx/test/compare/{base}...{head}",
            "merges_url": "https://api.github.com/repos/xxxxxx/test/merges",
            "archive_url": "https://api.github.com/repos/xxxxxx/test/{archive_format}{/ref}",
            "downloads_url": "https://api.github.com/repos/xxxxxx/test/downloads",
            "issues_url": "https://api.github.com/repos/xxxxxx/test/issues{/number}",
            "pulls_url": "https://api.github.com/repos/xxxxxx/test/pulls{/number}",
            "milestones_url": "https://api.github.com/repos/xxxxxx/test/milestones{/number}",
            "notifications_url": "https://api.github.com/repos/xxxxxx/test/notifications{?since,all,participating}",
            "labels_url": "https://api.github.com/repos/xxxxxx/test/labels{/name}",
            "releases_url": "https://api.github.com/repos/xxxxxx/test/releases{/id}",
            "created_at": 1729047958,
            "updated_at": "2024-10-21T09:46:22Z",
            "pushed_at": 1729507825,
            "git_url": "git://github.com/xxxxxx/test.git",
            "ssh_url": "[email protected]:xxxxxx/test.git",
            "clone_url": "https://github.com/xxxxxx/test.git",
            "svn_url": "https://github.com/xxxxxx/test",
            "homepage": null,
            "size": 29,
            "stargazers_count": 0,
            "watchers_count": 0,
            "language": null,
            "has_issues": true,
            "has_downloads": true,
            "has_wiki": true,
            "has_pages": false,
            "forks_count": 0,
            "mirror_url": null,
            "open_issues_count": 0,
            "forks": 0,
            "open_issues": 0,
            "watchers": 0,
            "default_branch": "main",
            "stargazers": 0,
            "master_branch": "main"
        },
        "pusher": {
            "name": "xxxxxx",
            "email": "[email protected]"
        },
        "sender": {
            "login": "xxxxxx",
            "id": 53037653,
            "node_id": "MDQ6VXNlcjUzMDM3NjUz",
            "avatar_url": "https://avatars.githubusercontent.com/u/53037653?v=4",
            "gravatar_id": "",
            "url": "https://api.github.com/users/xxxxxx",
            "html_url": "https://github.com/xxxxxx",
            "followers_url": "https://api.github.com/users/xxxxxx/followers",
            "following_url": "https://api.github.com/users/xxxxxx/following{/other_user}",
            "gists_url": "https://api.github.com/users/xxxxxx/gists{/gist_id}",
            "starred_url": "https://api.github.com/users/xxxxxx/starred{/owner}{/repo}",
            "subscriptions_url": "https://api.github.com/users/xxxxxx/subscriptions",
            "organizations_url": "https://api.github.com/users/xxxxxx/orgs",
            "repos_url": "https://api.github.com/users/xxxxxx/repos",
            "events_url": "https://api.github.com/users/xxxxxx/events{/privacy}",
            "received_events_url": "https://api.github.com/users/xxxxxx/received_events",
            "type": "User",
            "site_admin": false
        },
        "installation": {
            "id": 0
        }
    }
}



### Anything else?

_No response_

The text was updated successfully, but these errors were encountered:

0xavi0 · 2024-10-21T15:24:27Z

I can recreate the issue.
Problem is caused after we added the new WebhookCommit field in GitRepo recently.

Something like this should fix the issue.

diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
index 549ca958..06cba7e4 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
@@ -99,6 +99,7 @@ func (r *GitJobReconciler) SetupWithManager(mgr ctrl.Manager) error {
                                        predicate.GenerationChangedPredicate{},
                                        predicate.AnnotationChangedPredicate{},
                                        predicate.LabelChangedPredicate{},
+                                       webhookCommitChangedPredicate(),
                                ),
                        ),
                ).
@@ -1187,3 +1188,20 @@ func result(repoPolled bool, gitrepo *v1alpha1.GitRepo) reconcile.Result {
        }
        return reconcile.Result{}
 }
+
+func webhookCommitChangedPredicate() predicate.Predicate {
+       return predicate.Funcs{
+               UpdateFunc: func(e event.UpdateEvent) bool {
+                       oldGitJob, ok := e.ObjectOld.(*v1alpha1.GitRepo)
+                       if !ok {
+                               return true
+                       }
+                       newGitJob, ok := e.ObjectNew.(*v1alpha1.GitRepo)
+                       if !ok {
+                               return true
+                       }
+
+                       return oldGitJob.Status.WebhookCommit != newGitJob.Status.WebhookCommit
+               },
+       }
+}

I'd like to add extra tests, though.

This adds a new predicate when the webhook commit is changed in the `GitRepo`. It also filters out `Job` events for creation/deletion, because they add extra reconcile loops that only increase the possibility of race conditions and that are not required. The reconciler already knows when a `Job` is created/deleted because it is the owner and does not need to react upon those 2 events from the cluster. Refers to: rancher#2969 Signed-off-by: Xavi Garcia <[email protected]>

* Adds predicate when webhook commit changes This adds a new predicate when the webhook commit is changed in the `GitRepo`. It also filters out `Job` events for creation/deletion, because they add extra reconcile loops that only increase the possibility of race conditions and that are not required. The reconciler already knows when a `Job` is created/deleted because it is the owner and does not need to react upon those 2 events from the cluster. Refers to: #2969 --------- Signed-off-by: Xavi Garcia <[email protected]> Co-authored-by: Corentin Néau <[email protected]> Co-authored-by: Mario Manno <[email protected]>

* Adds predicate when webhook commit changes This adds a new predicate when the webhook commit is changed in the `GitRepo`. It also filters out `Job` events for creation/deletion, because they add extra reconcile loops that only increase the possibility of race conditions and that are not required. The reconciler already knows when a `Job` is created/deleted because it is the owner and does not need to react upon those 2 events from the cluster. Refers to: rancher#2969 --------- Signed-off-by: Xavi Garcia <[email protected]> Co-authored-by: Corentin Néau <[email protected]> Co-authored-by: Mario Manno <[email protected]>

* Adds predicate when webhook commit changes This adds a new predicate when the webhook commit is changed in the `GitRepo`. It also filters out `Job` events for creation/deletion, because they add extra reconcile loops that only increase the possibility of race conditions and that are not required. The reconciler already knows when a `Job` is created/deleted because it is the owner and does not need to react upon those 2 events from the cluster. Refers to: #2969 --------- Signed-off-by: Xavi Garcia <[email protected]> Co-authored-by: Corentin Néau <[email protected]> Co-authored-by: Mario Manno <[email protected]>

aDisplayName · 2024-10-28T14:36:59Z

This is related to #2972 .

The screenshot is taken around 9:21 am.
Commit b9be4 was submitted to azure devops at 9:09am
The job for that commit was carried out around 9:18 am, and it states the "GotNewCommit" event was also raised at the same time.

Here is the response from the gitjob in rancher server to the azure devops code commit webhook for the same commit.

Status Code: 200
Reason Phrase: OK
HTTP Version: 1.1
Headers:
{
  Connection: keep-alive
  Strict-Transport-Security: max-age=31536000; includeSubDomains
  Date: Mon, 28 Oct 2024 14:09:08 GMT
  Content-Length: 9
  Content-Type: text/plain; charset=utf-8
}

There is a significant delay between the actual code commit to the actual gitjob for gitrepo bundle update.

skanakal · 2024-10-30T07:11:09Z

@0xavi0 I have verified this fix again, and it seems to be working fine...
Thank you for your time and help on this...

mmartin24 · 2024-10-31T12:08:00Z

Tested in with Rancher 2.10.0-alpha5 and fleet:105.0.0+up0.11.0-beta.3 deployed in GCP and working:

Steps done:

Created Github webhook with SSL verification disabled and Payload URL with https
Deployed above mentioned rancher version with 3 ds clusters. Passed as ingress reverse order of GCP ip with nip.io. Detailed steps here: Configure Github Webhook and Fleet
Deployed private gitrepo (https://github.com/fleetqa/webhook-private-test) with disablePoling=true ensuring solely webhook update its status.
Changed on Gitrepo amount of replicas and checked if changed is propagated immediately. This can be seen here:

Screencast.from.31-10-24.12.50.03.mov

Checked also 200 response on Webhook response:

skanakal added the kind/bug label Oct 21, 2024

rancherbot added this to Fleet Oct 21, 2024

github-project-automation bot moved this to 🆕 New in Fleet Oct 21, 2024

skanakal changed the title ~~New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments~~ [SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments Oct 21, 2024

skanakal added the JIRA Must shout label Oct 21, 2024

manno moved this from 🆕 New to 📋 Backlog in Fleet Oct 22, 2024

manno added this to the v2.9.4 milestone Oct 22, 2024

manno added the priority/high label Oct 22, 2024

kkaempf assigned 0xavi0 Oct 22, 2024

kkaempf moved this from 📋 Backlog to 🏗 In progress in Fleet Oct 22, 2024

mmartin24 mentioned this issue Oct 22, 2024

Multiple gitjobs for one GitRepo, and gitrepo updates are not being triggered by future git code commit. #2972

Closed

1 task

0xavi0 mentioned this issue Oct 23, 2024

Adds predicate when webhook commit changes #2986

Merged

0xavi0 mentioned this issue Oct 24, 2024

[v0.10] Backport of [SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #3003

Closed

1 task

0xavi0 mentioned this issue Oct 24, 2024

[v0.10] Backport of Adds predicate when webhook commit changes #3006

Merged

0xavi0 modified the milestones: v2.9.4, v2.10.0 Oct 30, 2024

mmartin24 self-assigned this Oct 30, 2024

mmartin24 closed this as completed Oct 31, 2024

github-project-automation bot moved this from Needs QA review to ✅ Done in Fleet Oct 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #2969

[SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #2969

skanakal commented Oct 21, 2024

0xavi0 commented Oct 21, 2024

aDisplayName commented Oct 28, 2024

skanakal commented Oct 30, 2024

mmartin24 commented Oct 31, 2024

[SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #2969

[SURE-9190]New commits from webhook should trigger deployments, but disablePolling for gitrepo prevents new deployments #2969

Comments

skanakal commented Oct 21, 2024

Is there an existing issue for this?

Current Behavior

Expected Behavior

Steps To Reproduce

Environment

Logs

0xavi0 commented Oct 21, 2024

aDisplayName commented Oct 28, 2024

skanakal commented Oct 30, 2024

mmartin24 commented Oct 31, 2024