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 1756461: pkg/cvo/status: Don't clobber last.CompletionTime when adding new entries #247

Merged
merged 1 commit into from Oct 8, 2019
Merged

Bug 1756461: pkg/cvo/status: Don't clobber last.CompletionTime when adding new entries #247

merged 1 commit into from Oct 8, 2019

Conversation

wking
Copy link
Member

@wking wking commented Sep 30, 2019

Fix a bug where history[n-1].completionTime == history[n].startedTime. @smarterclayton pointed out this fix.

@wking
pkg/cvo/status: Don't clobber last.CompletionTime when adding new ent…
f81ba7a 
…ries

Fix a bug where history[n-1].completionTime == history[n].startedTime
[1].  Clayton pointed out this fix [2].

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1756461
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=1756461#c3
@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 Sep 30, 2019
@openshift-ci-robot
Copy link
Contributor

@wking: This pull request references Bugzilla bug 1756461, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1756461: pkg/cvo/status: Don't clobber last.CompletionTime when adding new entries

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 30, 2019
@wking
Copy link
Member Author

wking commented Sep 30, 2019

/cherrypick release-4.2
/cherrypick release-4.1

@openshift-cherrypick-robot
Copy link

@wking: once the present PR merges, I will cherry-pick it on top of release-4.2 in a new PR and assign it to you.

In response to this:

/cherrypick release-4.2
/cherrypick release-4.1

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 Sep 30, 2019

integration:

Waiting for kube-controller-manager to come up
!!! [0930
---

Dunno what that's about.

/test integration

@wking
Copy link
Member Author

wking commented Oct 2, 2019

Same error again. One more try before I dig deeper:

/retest

@abhinavdahiya
Copy link
Contributor

/retest

@abhinavdahiya
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,wking]

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

@openshift-merge-robot openshift-merge-robot merged commit bdb4644 into openshift:master Oct 8, 2019
@openshift-ci-robot
Copy link
Contributor

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

In response to this:

Bug 1756461: pkg/cvo/status: Don't clobber last.CompletionTime when adding new entries

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 openshift-cherrypick-robot mentioned this pull request Oct 8, 2019
Merged
@openshift-cherrypick-robot
Copy link

@wking: new pull request created: #250

In response to this:

/cherrypick release-4.2
/cherrypick release-4.1

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 Oct 8, 2019

/cherrypick release-4.1

@openshift-cherrypick-robot
Copy link

@wking: new pull request created: #251

In response to this:

/cherrypick release-4.1

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 wking deleted the stop-clobbering-completion-time branch October 8, 2019 21:35
@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 8, 2019
Merged
@wking
Copy link
Member Author

wking commented Oct 8, 2019

Still seeing the bug in the final e2e run here:

$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_cluster-version-operator/247/pull-ci-openshift-cluster-version-operator-master-e2e-aws-upgrade/314/artifacts/e2e-aws-upgrade/must-gather/registry-svc-ci-openshift-org-ci-op-9miqrbbj-stable-sha256-14991b2279a6beb45691151036bdca03b3423319ebad519a4679cde53095835d/cluster-scoped-resources/config.openshift.io/clusterversions/version.yaml | yaml2json | jq -r '.status.history[] | .startedTime + " " + .completionTime + " " + .state'
2019-10-08T20:44:42Z 2019-10-08T21:16:40Z Completed
2019-10-08T20:22:36Z 2019-10-08T20:44:42Z Completed

But the changes from this PR landed in 4.3.0-0.ci-2019-10-08-212413, so I'll check again once that or a later CI build gets promoted and used as an upgrade base.

@smarterclayton
Copy link
Contributor

I would have expected a unit test to show the problem and demonstrate it's fixed.

@wking
Copy link
Member Author

wking commented Oct 9, 2019 

$ curl -s https://storage.googleapis.com/origin-ci-test/logs/release-openshift-origin-installer-e2e-aws-upgrade/8135/artifacts/e2e-aws-upgrade/must-gather/registry-svc-ci-openshift-org-ocp-4-3-2019-10-09-004754-sha256-14991b2279a6beb45691151036bdca03b3423319ebad519a4679cde53095835d/cluster-scoped-resources/config.openshift.io/clusterversions/version.yaml | yaml2json | jq -r '.status.history[] | .startedTime + " " + .completionTime + " " + .state'
2019-10-09T01:30:51Z 2019-10-09T01:59:25Z Completed
2019-10-09T01:08:43Z 2019-10-09T01:26:36Z Completed

So that looks good :).

I would have expected a unit test to show the problem and demonstrate it's fixed.

Yeah, that would be good too :).

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

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@abhinavdahiya abhinavdahiya

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@wking @openshift-ci-robot @openshift-cherrypick-robot @abhinavdahiya @smarterclayton @openshift-merge-robot