search

Azure / acr

Azure Container Registry samples, troubleshooting tips and references
https://aka.ms/acr
Other
164 stars 109 forks source link

ACR Tasks with GitHub commit triggers should build newest commit #152

Closed gandro closed 5 years ago

gandro commented 5 years ago

Is this a BUG REPORT or FEATURE REQUEST?:

Bug report.

What happened?:

We created an ACR build task with the following command (according to this guide).

az acr task create --registry acrname --name taskname --image imagename:latest --context https://github.com/orga/repo.git --branch develop --file Dockerfile --git-access-token token

When pushing commits to the GitHub remote, a new run is started as expected. However, if if multiple commits were pushed (e.g. the result of a git merge), the ACR builds the oldest commit:

Pushed commits:

73092c8 Merge branch 'master' into develop
3ff61d5 Merge branch 'master' of https://github.com/orga/repo.git 
ee39b7d fix conflicts in cleaner when running into recurring timer

Metadata of run:

az acr task show-run --run-id cbgs --output json 
{
  "agentConfiguration": {
    "cpu": 2
  },
  "createTime": "2018-11-08T13:38:09.796413+00:00",
  "finishTime": "2018-11-08T13:41:49.481555+00:00",
  "id": "/subscriptions/93a99202-4ca1-44d4-9d87-3f406f012885/resourceGroups/devcloud/providers/Microsoft.ContainerRegistry/registries/acrname/runs/cbgs",
  "imageUpdateTrigger": null,
  "isArchiveEnabled": false,
  "lastUpdatedTime": "2018-11-08T13:41:49+00:00",
  "name": "cbgs",
  "outputImages": [
    {
      "digest": "sha256:f974167808eac9a09a2b4be0d1bb5c1d7d3ddaefc71372c7fe9c62a2c021d966",
      "registry": "acrname.azurecr.io",
      "repository": "imagename",
      "tag": "latest"
    }
  ],
  "platform": {
    "architecture": "amd64",
    "os": "Linux",
    "variant": null
  },
  "provisioningState": "Succeeded",
  "resourceGroup": "rg",
  "runId": "cbgs",
  "runType": "AutoRun",
  "sourceTrigger": {
    "branchName": "develop",
    "commitId": "ee39b7da1135111f902f22903f1fe1ffbffffffff",
    "eventType": "Commit",
    "id": "876c0524-e35b-11e8-89f4-9fba6973ca02",
    "providerType": "GitHub",
    "pullRequestId": null,
    "repositoryUrl": "https://github.com/orga/repo"
  },
  "startTime": "2018-11-08T13:38:10.635144+00:00",
  "status": "Succeeded",
  "task": "taskname",
  "type": "Microsoft.ContainerRegistry/registries/runs"
}

Note that the commitId is ee39b7d - which is the oldest of the pushed commits.

What did you expect to happen?:

An ACR task triggered by a git push which contained multiple commits should build the newest commit, e.g. the merge commit 73092c8 in this example.

How do you reproduce it (as minimally and precisely as possible)?:

➜ git checkout develop
➜ git merge master
Merge made by the 'recursive' strategy.
 pkg/package/file.go | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

➜ git push
Enumerating objects: 10, done.
Counting objects: 100% (10/10), done.
Delta compression using up to 8 threads
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 418 bytes | 418.00 KiB/s, done.
Total 4 (delta 3), reused 0 (delta 0)
remote: Resolving deltas: 100% (3/3), completed with 3 local objects.
To https://github.com/orga/repo.git
   2f7893d..73092c8  develop -> develop

➜ git log --oneline --decorate= 2f7893d..73092c
73092c8 Merge branch 'master' into develop                         <-- this one *SHOULD* be built
3ff61d5 Merge branch 'master' of https://github.com/monostream/devcloud.builder
ee39b7d fix conflicts in cleaner when running into recurring timer <-- this one is built

See above. Make multiple git commits locally and push them together to a GitHub remote. The triggered ACR task will build the oldest commit ee39b7d instead of 73092c8.

ehotinger commented 5 years ago

@northtyphoon @mnltejaswini

mnltejaswini commented 5 years ago

@gandro Can you please share what CLI version are you using.

mnltejaswini commented 5 years ago

@jaysterp Can you please help with this issue

gandro commented 5 years ago

@mnltejaswini I can reproduce the issue with the following version:

az --version
azure-cli (2.0.50)

acr (2.1.8)
acs (2.3.11)
advisor (2.0.0)
ams (0.3.0)
appservice (0.2.6)
backup (1.2.1)
batch (3.4.1)
batchai (0.4.4)
billing (0.2.0)
botservice (0.1.1)
cdn (0.2.0)
cloud (2.1.0)
cognitiveservices (0.2.3)
command-modules-nspkg (2.0.2)
configure (2.0.19)
consumption (0.4.0)
container (0.3.8)
core (2.0.50)
cosmosdb (0.2.3)
dla (0.2.3)
dls (0.1.4)
dms (0.1.1)
eventgrid (0.2.0)
eventhubs (0.3.1)
extension (0.2.3)
feedback (2.1.4)
find (0.2.12)
hdinsight (0.1.0)
interactive (0.4.0)
iot (0.3.4)
iotcentral (0.1.3)
keyvault (2.2.6)
lab (0.1.3)
maps (0.3.2)
monitor (0.2.6)
network (2.2.8)
nspkg (3.0.3)
policyinsights (0.1.0)
profile (2.1.2)
rdbms (0.3.4)
redis (0.3.2)
relay (0.1.2)
reservations (0.4.0)
resource (2.1.6)
role (2.1.9)
search (0.1.1)
servicebus (0.3.2)
servicefabric (0.1.7)
signalr (1.0.0)
sql (2.1.5)
storage (2.2.4)
telemetry (1.0.0)
vm (2.2.7)

Python location '/home/gandro/.local/lib/azure-cli/bin/python'
Extensions directory '/home/gandro/.azure/cliextensions'

Python (Linux) 3.7.1 (default, Oct 22 2018, 10:41:28) 
[GCC 8.2.1 20180831]

Legal docs and information: aka.ms/AzureCliLegal
mnltejaswini commented 5 years ago

@gandro Just wanted to confirm the acrname and taskname that you gave in the above logs are not the actual names right?

gandro commented 5 years ago

@mnltejaswini Yes, I've anonymized the names :)

jaysterp commented 5 years ago

@gandro We repro'd this issue and found that this is a bug in the code and we will roll out a fix soon. Thank you!

gandro commented 5 years ago

:tada: Awesome, thank you!

jaysterp commented 5 years ago

@gandro Here is some info on the root cause of this issue: If you have an ACR task with a GitHub commit trigger and push multiple commits once, GitHub sends a list of commit ids such as

{
  "commits": [
    {
      "id": "16cc7564eefe7c21a094963b8dddf60028a00804",
      "tree_id": "03414b5fbe7903943bd7b958476a97cc5d1b34m5",
      "distinct": true,
      "message": "commit1",
      "timestamp": "2018-11-09T11:32:05-08:00",
      "url": "https://github.com/testuser/builddemo/commit/16cc7564eefe7c21a094963b8dddf60028a00808",
      "author": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "committer": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "added": [],
      "removed": [],
      "modified": [
        "test.txt"
      ]
    },
    {
      "id": "43c0ff8407e63cd477405c11144301fd53446653",
      "tree_id": "70b2f8e479c3682a08a80cc8afdfc2eb379785de",
      "distinct": true,
      "message": "commit2",
      "timestamp": "2018-11-09T11:32:35-08:00",
      "url": "https://github.com/testuser/builddemo/commit/43c0ff8407e63cd477405c11144301fd53445531",
      "author": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "committer": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "added": [],
      "removed": [],
      "modified": [
        "hello.txt"
      ]
    },
    {
      "id": "0db4830ddb85a1eccb17c9a59d9dce34g90ad462",
      "tree_id": "2874a34a7365e164381922e95c7f9c41c3fc9545",
      "distinct": true,
      "message": "commit3",
      "timestamp": "2018-11-09T11:32:49-08:00",
      "url": "https://github.com/testuser/builddemo/commit/0db4830ddb85a1eccb17c9a59d9dce32e90ad462",
      "author": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "committer": {
        "name": "Test User",
        "email": "testuser@microsoft.com",
        "username": "testuser"
      },
      "added": [],
      "removed": [],
      "modified": [
        "test.txt"
      ]
    }
  ]
}

GitHub sends this list of commits in chronological order. In the above example commit3 is the most recent commit. However, we parse this payload by looking at the first commit id in the list

CommitId = (string)triggerBodyJObject["commits"]?.First?["id"]

For GitHub, this means that we incorrectly trigger a build using the oldest commit. ACR also supports triggers in Azure DevOps, which sends a list of commit ids in the opposite order than GitHub (i.e. newest commit first). This is why we initially used the above method of parsing. We are making a fix so that in the case of GitHub commit triggers, we use .Last instead of .First so that we capture the most recent commit for an image build. Thank you for letting us know :)

jaysterp commented 5 years ago

@gandro We released a fix for this bug. Can you please confirm that you are unblocked?

gandro commented 5 years ago

@jaysterp Perfect, works as expected now. Thank you!