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.

Sign up for GitHub

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

Jump to bottom

Bug 1805155: Fix image resolve plugin on updates and add tests #24530

Merged
merged 2 commits into from Feb 20, 2020
Merged

Bug 1805155: Fix image resolve plugin on updates and add tests #24530

merged 2 commits into from Feb 20, 2020

Conversation

tnozicka
Copy link
Contributor

@tnozicka tnozicka commented Feb 12, 2020 •
edited

These were not covered before.

Requires:

/cc @soltysh @adambkaplan @smarterclayton

/hold
(I'll like to sync merging it with our decision on fixing oc set image-lookup which sets the annotation only for the template.)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2020
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 12, 2020
@soltysh
Copy link
Member

soltysh commented Feb 12, 2020

(I'll like to sync merging it with our decision on fixing oc set image-lookup which sets the annotation only for the template.)

All of our higher-level resources (those that embed PodTemplate) set it on PodTemplate, so it's consistent, I don't think we need to change it.

adambkaplan
adambkaplan approved these changes Feb 12, 2020
View reviewed changes
Copy link
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

looks good to me, allowing others to weigh in.

@tnozicka
Copy link
Contributor Author

(I'll like to sync merging it with our decision on fixing oc set image-lookup which sets the annotation only for the template.)

All of our higher-level resources (those that embed PodTemplate) set it on PodTemplate, so it's consistent, I don't think we need to change it.

Mutating admission that changes only a RS and not the parent resource template (like Deployment) is gonna set any workload on fire (hot looping) as they are based on comparing the template with the RS.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 13, 2020
@tnozicka tnozicka changed the title Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet [WIP] Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet Feb 13, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2020
@tnozicka tnozicka mentioned this pull request Feb 14, 2020
Merged
@soltysh soltysh mentioned this pull request Feb 14, 2020
Closed
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. vendor-update Touching vendor dir or related files and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 14, 2020
@tnozicka tnozicka changed the title [WIP] Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet [WIP] Bug 1801095: Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet Feb 14, 2020
@openshift-ci-robot
Copy link

@tnozicka: This pull request references Bugzilla bug 1801095, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

[WIP] Bug 1801095: Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 14, 2020
@tnozicka
Copy link
Contributor Author

/retest

1 similar comment
@tnozicka
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

13 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 3fc494b into openshift:master Feb 20, 2020
@tnozicka tnozicka deleted the fix-resolve-images branch February 20, 2020 08:07
@tnozicka
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot
Copy link

@tnozicka: All pull requests linked via external trackers have merged. Bugzilla bug 1801095 has been moved to the MODIFIED state.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tnozicka tnozicka changed the title Bug 1801095: Add e2e image resolve test for Deployment, DaemonSet adn StatefulSet Bug 1801095: Fix image resolve plugin on updates and add tests Feb 20, 2020
@tnozicka
Copy link
Contributor Author

/cherry-pick release-4.4

@tnozicka
Copy link
Contributor Author

/cherrypick release-4.4

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Feb 20, 2020
Merged
@openshift-cherrypick-robot
Copy link

@tnozicka: new pull request created: #24571

In response to this:

/cherry-pick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot
Copy link

@tnozicka: failed to push cherry-picked changes in GitHub: pushing failed, output: "To https://github.com/openshift-cherrypick-robot/origin\n ! [rejected] cherry-pick-24530-to-release-4.4 -> cherry-pick-24530-to-release-4.4 (fetch first)\nerror: failed to push some refs to 'https://openshift-cherrypick-robot:CENSORED@github.com/openshift-cherrypick-robot/origin'\nhint: Updates were rejected because the remote contains work that you do\nhint: not have locally. This is usually caused by another repository pushing\nhint: to the same ref. You may want to first integrate the remote changes\nhint: (e.g., 'git pull ...') before pushing again.\nhint: See the 'Note about fast-forwards' in 'git push --help' for details.\n", error: exit status 1

In response to this:

/cherrypick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tnozicka tnozicka changed the title Bug 1801095: Fix image resolve plugin on updates and add tests Bug 1805155: Fix image resolve plugin on updates and add tests Feb 20, 2020
@openshift-ci-robot
Copy link

@tnozicka: All pull requests linked via external trackers have merged. Bugzilla bug 1805155 has been moved to the MODIFIED state.

In response to this:

Bug 1805155: Fix image resolve plugin on updates and add tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tnozicka
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot
Copy link

@tnozicka: Bugzilla bug 1805155 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tnozicka
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot
Copy link

@tnozicka: Bugzilla bug 1805155 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soltysh soltysh soltysh approved these changes

@adambkaplan adambkaplan adambkaplan approved these changes

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@soltysh soltysh

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. vendor-update Touching vendor dir or related files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@tnozicka @soltysh @openshift-ci-robot @openshift-bot @openshift-cherrypick-robot @adambkaplan @openshift-merge-robot