Correctly handle NotFound errors during migration #17080
Conversation
openshift-ci-robot
added
the
kind/bug
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: enj Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-ci-robot
added
the
size/M
|
/retest |
1 similar comment
|
/retest |
|
@enj can this be opened on stage as well? |
pkg/oc/admin/migrate/migrator.go
Outdated
| @@ -49,6 +50,11 @@ func AlwaysRequiresMigration(_ *resource.Info) (Reporter, error) { | |||
| return ReporterBool(true), nil | |||
| } | |||
|
|
|||
| // TimeStampNow returns the current time in the same format as glog | |||
| func TimeStampNow() string { | |||
There was a problem hiding this comment.
make this private and add a test, otherwise LGTM
There was a problem hiding this comment.
Exactly what should I add a test for?
There was a problem hiding this comment.
Add a test for your change.
This change makes it so that the migrate storage command will now reprocess the body of a failed GET request to follow the standard path of extracting a status error. This normalizes the structure of the errors the code must handle. When comparing the resource.Info to an error, the info's REST mapping is now used to extract group kind information since the associated runtime.Object is not guaranteed to have valid type meta data. This is combined with relaxed case-insensitive matching to make sure that we only fail the migrate storage command on NotFound errors that we know do not match the info. When a NotFound error occurs, the migrate command now correctly reports that it did not change the resource. Previously it reported that it had successfully migrated the resource, which is not true since it is impossible to migrate a resource that does not exist. Bug 1506006 Signed-off-by: Monis Khan <[email protected]>
This change adds glog style timestamps to the reporting output of the migration tracker. This will aid in debugging migration errors by making it easier to correlate client errors with master logs. Signed-off-by: Monis Khan <[email protected]>
enj
force-pushed
the
enj/i/migrate_not_found_doraw_/1506006
branch
from
ffe93be to
a5b902e
Compare
openshift-ci-robot
added
size/L
enj
added
approved
|
Comments addressed, tagging. /retest |
|
/test extended_conformance_gce |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Automatic merge from submit-queue. |
…stream-stage Automated cherry pick of #17080
This change makes it so that the migrate storage command will now reprocess the body of a failed GET request to follow the standard path of extracting a status error. This normalizes the structure of the errors the code must handle.
When comparing the resource.Info to an error, the info's REST mapping is now used to extract group kind information since the associated runtime.Object is not guaranteed to have valid type meta data. This is combined with relaxed case-insensitive matching to make sure that we only fail the migrate storage command on NotFound errors that we know do not match the info.
When a NotFound error occurs, the migrate command now correctly reports that it did not change the resource. Previously it reported that it had successfully migrated the resource, which is not true since it is impossible to migrate a resource that does not exist.
Bug 1506006
Signed-off-by: Monis Khan [email protected]
Add timestamps to migration command's reporting
This change adds glog style timestamps to the reporting output of the migration tracker. This will aid in debugging migration errors by making it easier to correlate client errors with master logs.
Signed-off-by: Monis Khan [email protected]
/assign @smarterclayton
/kind bug
xref https://bugzilla.redhat.com/show_bug.cgi?id=1506006
@sdodson @jupierce