Volume command doesn't retry on conflicts

[liggitt]

oc volume dc/test-deployment-config --remove --confirm

error: deploymentConfig "test-deployment-config" cannot be updated: the object has been modified; please apply your changes to the latest version and try again
!!! Error in test/cmd/volumes.sh:29
  '[ "$(oc volume dc/test-deployment-config --remove --confirm)" ]' exited with status 1
Call stack:
  1: test/cmd/volumes.sh:29 main(...)
Exiting with status 1
!!! Error in hack/test-cmd.sh:269
  '${test}' exited with status 1
Call stack:
  1: hack/test-cmd.sh:269 main(...)

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/4461/console

[liggitt]

Not sure if it should retry, but failure is making cmd tests flake

[liggitt]

I think most commands that don't take a resourceVersion as input, and that fetch and immediately update, should retry at least once if they get a conflict. If the user didn't provide a resourceVersion as input, then they are requesting a bounded mutation which the command should attempt to fulfil.

This is especially important on resources that get modified by controller loops (deployment configs, replication controllers, pods, etc).

Example commands (when not provided a resourceVersion as input):

oc label
oc annotate
oc volume
oc scale?
oc env?
oc start-build?
oc deploy?

[smarterclayton]

I thought we opened an upstream issue for this

[smarterclayton]

Things we can check

• generation
• spec differences
• metadata differences
  • knowing what metadata we changed

A generalized pattern would be better than a specific one.

[smarterclayton]

I'm wondering if we should now be using Patch on the server for this.

[liggitt]

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6679/console

error: deploymentConfig "test-deployment-config" cannot be updated: the object has been modified; please apply your changes to the latest version and try again
!!! Error in test/cmd/volumes.sh:17
    '[ "$(oc volume dc/test-deployment-config --add --name=vol2 --type=emptydir -m /opt)" ]' exited with status 1
Call stack:
    1: test/cmd/volumes.sh:17 main(...)

[smarterclayton]

Post 1.1

[childsb]

Does this problem still exist?

[liggitt]

Yes, the volume command hasn't switched to use patch yet, so it is still subject to resource conflicts.

[bparees]

not sure if this is the same issue or not, but:

=== BEGIN TEST CASE ===
test/cmd/volumes.sh:48: executing 'oc set volume dc/test-deployment-config --add --mount-path=/second --type=pvc --claim-size=1G --claim-mode=rwo' expecting success
FAILURE after 0.759s: test/cmd/volumes.sh:48: executing 'oc set volume dc/test-deployment-config --add --mount-path=/second --type=pvc --claim-size=1G --claim-mode=rwo' expecting success: the command returned the wrong error code
Standard output from the command:
persistentvolumeclaims/pvc-ilqx4

Standard error from the command:
info: Generated volume name: volume-l6n9f
error: Operation cannot be fulfilled on deploymentconfigs "test-deployment-config": the object has been modified; please apply your changes to the latest version and try again
=== END TEST CASE ===

as seen in:
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_check/2669/console

[smarterclayton]

It is indeed

On Jun 30, 2016, at 6:02 AM, Ben Parees notifications@github.com wrote:

not sure if this is the same issue or not, but:

=== BEGIN TEST CASE ===
test/cmd/volumes.sh:48: executing 'oc set volume
dc/test-deployment-config --add --mount-path=/second --type=pvc
--claim-size=1G --claim-mode=rwo' expecting success
FAILURE after 0.759s: test/cmd/volumes.sh:48: executing 'oc set volume
dc/test-deployment-config --add --mount-path=/second --type=pvc
--claim-size=1G --claim-mode=rwo' expecting success: the command
returned the wrong error code
Standard output from the command:
persistentvolumeclaims/pvc-ilqx4

Standard error from the command:
info: Generated volume name: volume-l6n9f
error: Operation cannot be fulfilled on deploymentconfigs
"test-deployment-config": the object has been modified; please apply
your changes to the latest version and try again
=== END TEST CASE ===

as seen in:
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_check/2669/console

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#4351 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABG_p28W0QmIbDPngcVdN78xuC-I8NV_ks5qQ75bgaJpZM4FxdH7
.

[deads2k]

I thought we opened an upstream issue for this

We switched the most common offenders to use patch which allowed them to automatically reapply non-conflicting changes. A blind retryOnConflict can cause concurrent conflicting changes to unconditionally stomp each other which seems less than desirable.

[bparees]

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_check/2995/consoleFull

[gnufied]

I emailed @liggitt about this, but one potential problem I have been grappling with is - this command can't be entirely moved to use PATCH because it allows user to create volume claims when adding volumes to Deploymentconfig etcetra.

So I am thinking, this has to be done in two steps:

1. if request includes adding volume claims - it will still do the usual POST
2. For all other cases it will do PATCH.

Does it make sense? @smarterclayton @childsb

[smarterclayton]

It's ok to create claims first with a post then apply a patch in a consistent code for changing the template. After all, we don't support atomic operations anyway.