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.

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

Bug 1860921: [release-4.6] Bug 1860921: Check also Pod status before enabling Fast upload #132

Merged

Conversation

martinkunc
Copy link
Contributor

@martinkunc martinkunc commented Jul 3, 2020

There are three main changes here:

  • Besides checking reported operator status, before enabling Fast upload path (without initial delay) it is also checking insights Operator Pod status. This is to address Bug 1841057: Skip the initial upload delay #117 (comment).
  • Originally, the fast upload mode was enabled and disabled by UpdateStatus, which is happening on startup and then periodically every 30s. On larger clusters when data Gathering took more then 30 seconds, this consequent call to updateStatus was not considered initial and disabled - even if operator was helthy. To fix this, fast mode is now disabled from after insights upload happens (successfully or not)..
  • Additional fast upload mode improvement - originally when Cluster Version wasnt available (which is collected during Gather) upload sets waiting time to random interval 0 - 15m, which could slow down initial fast upload. To fix this, when we are in fast upload mode, it is now waiting only 15 sec (to wait for Gather to finish).

@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 3, 2020
@martinkunc
Copy link
Contributor Author

/test insights-operator-e2e-tests

@martinkunc
Copy link
Contributor Author

/test e2e-aws
/test insights-operator-e2e-tests

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

It looks good, thank you @martinkunc

@tisnik
Copy link
Contributor

tisnik commented Jul 8, 2020

/lgtm

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: martinkunc, tisnik

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:

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

@openshift-cherrypick-robot

@martinkunc: #132 failed to apply on top of branch "release-4.2":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	pkg/controller/operator.go
M	pkg/controller/status/status.go
M	pkg/insights/insightsuploader/insightsuploader.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/insights/insightsuploader/insightsuploader.go
CONFLICT (content): Merge conflict in pkg/insights/insightsuploader/insightsuploader.go
Auto-merging pkg/controller/status/status.go
CONFLICT (content): Merge conflict in pkg/controller/status/status.go
Auto-merging pkg/controller/operator.go
Patch failed at 0001 Check also Pod status before enabling Fast upload. Fast status is now disabled only after upload.

In response to this:

/cherrypick release-4.2

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.

@martinkunc
Copy link
Contributor Author

/cherrypick release-4.5

@openshift-cherrypick-robot

@martinkunc: new pull request created: #139

In response to this:

/cherrypick release-4.5

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.

@martinkunc
Copy link
Contributor Author

/retitle Bug 1860921: [release-4.6] Bug 1860921: Check also Pod status before enabling Fast upload

@openshift-ci-robot openshift-ci-robot changed the title Check also Pod status before enabling Fast upload Bug 1860921: [release-4.6] Bug 1860921: Check also Pod status before enabling Fast upload Jul 27, 2020
@openshift-ci-robot
Copy link
Contributor

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

In response to this:

Bug 1860921: [release-4.6] Bug 1860921: Check also Pod status before enabling Fast upload

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
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
Development

Successfully merging this pull request may close these issues.

6 participants