Skip to content

[release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec#782

Merged
openshift-merge-robot merged 3 commits intoopenshift:release-4.4from
marun:apps-apply-spec-hash
May 12, 2020
Merged

[release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec#782
openshift-merge-robot merged 3 commits intoopenshift:release-4.4from
marun:apps-apply-spec-hash

Conversation

@marun
Copy link
Contributor

@marun marun commented Apr 23, 2020

Cherrypick of #773. Did not apply cleanly due to the addition of context to all client calls in master.

/cc @deads2k

marun added 3 commits April 23, 2020 00:18
@marun
Fix Apply{Deployment,Deployment} to copy input before mutatating
bb20204
@marun
Update ApplyDeployment to rely on a hash of the spec
c5a202b 
This removes the need for the caller to be able to force a
rollout. Any state that is not present in the spec should instead be
added as an annotation so that a rollout will occur when the external
state changes.

This is an intentional breaking change to encourage switching to the
revised function. Callers who want to continue using the
previous (deprecated) implementation will need to change the name of
the function they are calling to ApplyDeploymentWithForce.
@marun
Update ApplyDaemonSet to rely on a hash of the spec
bbb9bbd 
This removes the need for the caller to be able to force a
rollout. Any state that is not present in the spec should instead be
added as an annotation so that a rollout will occur when the external
state changes.

This is an intentional breaking change to encourage switching to the
revised function. Callers who want to continue using the
previous (deprecated) implementation will need to change the name of
the function they are calling to ApplyDaemonSetWithForce.
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Apr 23, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 23, 2020

@marun: This pull request references Bugzilla bug 1827052, which is invalid:

  • expected the bug to target the "4.4.0" release, but it targets "4.3.z" instead
  • expected dependent Bugzilla bug 1827050 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ASSIGNED instead
  • expected dependent Bugzilla bug 1827051 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ASSIGNED instead
  • expected dependent Bugzilla bug 1827050 to target a release in 4.5.0, 4.5.z, but it targets "---" instead
  • expected dependent Bugzilla bug 1827051 to target a release in 4.5.0, 4.5.z, but it targets "4.4.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1827052: Update Apply{DaemonSet,Deployment} to rely on a hash of the spec

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.

@marun marun changed the title Bug 1827052: Update Apply{DaemonSet,Deployment} to rely on a hash of the spec Bug 1827051: Update Apply{DaemonSet,Deployment} to rely on a hash of the spec Apr 23, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 23, 2020

@marun: This pull request references Bugzilla bug 1827051, which is invalid:

  • expected the bug to target the "4.4.0" release, but it targets "4.4.z" instead
  • expected dependent Bugzilla bug 1827050 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ASSIGNED instead
  • expected dependent Bugzilla bug 1827050 to target a release in 4.5.0, 4.5.z, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1827051: Update Apply{DaemonSet,Deployment} to rely on a hash of the spec

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.

@marun marun changed the title Bug 1827051: Update Apply{DaemonSet,Deployment} to rely on a hash of the spec Bug 1827051: [release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec Apr 23, 2020
@marun marun changed the title Bug 1827051: [release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec [release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec Apr 23, 2020
@openshift-ci-robot openshift-ci-robot removed the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Apr 23, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 23, 2020

@marun: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

[release-4.4] Update Apply{DaemonSet,Deployment} to rely on a hash of the spec

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 removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Apr 23, 2020
@deads2k
Copy link
Contributor

deads2k commented Apr 23, 2020

This has an impact to how the log level modification works. Without this, log level bumps (and other spec only changes) may not get applied.

/approve

leave lgtm to @sttts @mfojtik or @soltysh

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2020
@mfojtik
Copy link
Contributor

mfojtik commented Apr 23, 2020

/lgtm
/hold

patch manager note: please pick this for 4.4.1 (or early 4.4.z release).

@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 Apr 23, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 23, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, marun, mfojtik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2020
@mfojtik
Copy link
Contributor

mfojtik commented May 12, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2020
@openshift-merge-robot openshift-merge-robot merged commit 4e977da into openshift:release-4.4 May 12, 2020
@marun marun deleted the apps-apply-spec-hash branch May 20, 2020 14:33
@marun
Copy link
Contributor Author

marun commented May 20, 2020

/cherry-pick release-4.3

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

openshift-cherrypick-robot commented May 20, 2020

@marun: new pull request created: #801

Details

In response to this:

/cherry-pick release-4.3

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

@deads2k deads2k Awaiting requested review from deads2k

Assignees

@mfojtik mfojtik

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@marun @openshift-ci-robot @deads2k @mfojtik @openshift-cherrypick-robot @openshift-merge-robot