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

lib/resourcebuilder/apps: Explicit mode instead of 'Generation > 1' #401

Merged
merged 1 commit into from Jul 11, 2020
Merged

lib/resourcebuilder/apps: Explicit mode instead of 'Generation > 1' #401

merged 1 commit into from Jul 11, 2020

Conversation

wking
Copy link
Member

@wking wking commented Jul 8, 2020

deploymentBuilder and daemonsetBuilder grow mode properties. They had been using actual.Generation > 1 as a proxy for "post-install" since 14fab0b (#26), but generation 1 is just "we haven't changed the object since it was created", not "we're installing a fresh cluster". For example, a new Deployment or DaemonSet could be added as part of a cluster update, and we don't want special install-time "we don't care about specific manifest failures" then.

Spun out from #400 at @smarterclayton's request.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2020
@wking wking mentioned this pull request Jul 8, 2020
Merged
@wking
lib/resourcebuilder/apps: Explicit mode instead of 'Generation > 1'
844c3aa 
deploymentBuilder and daemonsetBuilder grow mode properties.  They had
been using 'actual.Generation > 1' as a proxy for "post-install" since
14fab0b (add generic 2-way merge handler for random types,
2018-09-27, openshift#26), but generation 1 is just "we haven't changed the
object since it was created", not "we're installing a fresh cluster".
For example, a new Deployment or DaemonSet could be added as part of a
cluster update, and we don't want special install-time "we don't care
about specific manifest failures" then.
@wking wking force-pushed the explicit-app-install-mode branch from 1e38ab0 to 844c3aa Compare July 8, 2020 01:41
@jottofar
Copy link
Contributor

jottofar commented Jul 8, 2020

/lgtm

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

/retest

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

2 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.

@wking
Copy link
Member Author

wking commented Jul 9, 2020

Service disruption

/retest

@openshift-bot
Copy link
Contributor

/retest

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

16 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-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.

21 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-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.

@wking
Copy link
Member Author

wking commented Jul 11, 2020

/override e2e-gcp-upgrade

We don't need that anymore, it's now the genrically-named e2e-upgrade.

@openshift-ci-robot
Copy link
Contributor

@wking: /override requires a failed status context to operate on.
The following unknown contexts were given:

  • e2e-gcp-upgrade

Only the following contexts were expected:

  • ci/prow/e2e
  • ci/prow/e2e-aws
  • ci/prow/e2e-aws-upgrade
  • ci/prow/e2e-gcp
  • ci/prow/e2e-gcp-upgrade
  • ci/prow/e2e-upgrade
  • ci/prow/images
  • ci/prow/integration
  • ci/prow/unit
  • tide

In response to this:

/override e2e-gcp-upgrade

We don't need that anymore, it's now the genrically-named e2e-upgrade.

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.

@wking
Copy link
Member Author

wking commented Jul 11, 2020

/override ci/prow/e2e-gcp-upgrade

@openshift-ci-robot
Copy link
Contributor

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-gcp-upgrade

In response to this:

/override ci/prow/e2e-gcp-upgrade

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-merge-robot openshift-merge-robot merged commit 2c00744 into openshift:master Jul 11, 2020
@wking wking deleted the explicit-app-install-mode branch July 11, 2020 14:18
@sdodson
Copy link
Member

sdodson commented Jul 11, 2020

/override e2e-gcp-upgrade

We don't need that anymore, it's now the genrically-named e2e-upgrade.

I think /skip is what's meant to be used when tests are removed or made non blocking but I guess that worked too.

@wking
Copy link
Member Author

wking commented Jul 11, 2020

I tried /skip up here, but maybe that was before the e2e-gcp -> e2e change.

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

@LalatenduMohanty LalatenduMohanty LalatenduMohanty approved these changes

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

Assignees

@LalatenduMohanty LalatenduMohanty

@jottofar jottofar

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.

None yet
7 participants
@wking @jottofar @openshift-bot @openshift-ci-robot @sdodson @LalatenduMohanty @openshift-merge-robot