Fix image change trigger imageRepo matching #858
Conversation
|
Please take a close look at this one. I want to make sure I have all the trigger/image repository field precedence rules correct. I made what I think is a pretty comprehensive table test. Resolves #838 as a side-effect. |
|
[test] |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1002/) |
There was a problem hiding this comment.
return repo.Namespace == namespace && repo.Name == trigger.From.Name
ironcladlou
force-pushed
the
image-trigger-matching-fix
branch
3 times, most recently
from
b0cbaa9 to
a0f1c48
Compare
There was a problem hiding this comment.
Invert this - do the simple check "empty" and return false, then do the more complex check in the final block.
ironcladlou
force-pushed
the
image-trigger-matching-fix
branch
from
a0f1c48 to
4d9b25d
Compare
|
Okay, ready for another look. Had to redo some of the tests. Just verified everything still works in e2e. |
There was a problem hiding this comment.
split this into two ifs - the first if should be "not trigger on image change || not automatic" continue. Make this easier to read.
ironcladlou
force-pushed
the
image-trigger-matching-fix
branch
2 times, most recently
from
6bd6ae6 to
6ac2444
Compare
|
Rebase to get rid of merge commit |
When matching deployment image change triggers to new image repository records, use correct matching logic which takes into account both the From and RepositoryName trigger fields.
ironcladlou
force-pushed
the
image-trigger-matching-fix
branch
from
41f0efe to
8d835b4
Compare
|
Rebased. |
|
LGTM 👍 |
|
LGTM too [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/844/) (Image: devenv-fedora_720) |
|
Evaluated for origin up to 8d835b4 |
4 participants
When matching deployment image change triggers to new image repository
records, use correct matching logic which takes into account both the
From and RepositoryName trigger fields.