Bug 1351127
Summary: | "--record=false" does not work for oc sub-commands such as oc edit | ||
---|---|---|---|
Product: | OKD | Reporter: | Zhang Cheng <chezhang> |
Component: | oc | Assignee: | Mike Dame <mdame> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Xingxing Xia <xxia> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | 3.x | CC: | aos-bugs, ffranz, mdame, mmccomas, pweil |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-09-19 13:50:42 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Zhang Cheng
2016-06-29 10:05:54 UTC
I believe this is a bug with kubernetes and how it records these changes. This issue only shows up when "--record=false" is passed to a command after a previous command used "--record=true". The code I'm referring to is here: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/util/helpers.go#L484-L495 Those functions decide if the change should be recorded, by checking if either the "record" flag is set, OR there is a ChangeCauseAnnotation set on the object. The problem arises if the flag had previously been set to "true", which records a ChangeCauseAnnotation and thus further prevents this logic check from ever failing. This could be fixed by changing the ShouldRecord function to only check for the "record" flag, but I don't know enough about the ChangeCauseAnnotations to know the importance of having an OR check in that spot. Other option for a proposed fix: https://github.com/damemi/kubernetes/commit/7b842faaf48e5839206f401972068a3ac8b71501 Mike, are you working on a fix? If so, please take assignment of the bug, tks! Yes, I submitted a PR here: https://github.com/kubernetes/kubernetes/pull/28234 Ok. Once the fix is accepted upstream you have to create a separate PR to OpenShift Origin following the UPSTREAM commit convention: https://github.com/openshift/origin/blob/master/HACKING.md#cherry-picking-an-upstream-commit-into-origin looks like this PR was accepted upstream, do we have the pick downstream? PR created: https://github.com/openshift/origin/pull/10486 Verified and Passed in openshift origin: $ oc version oc v1.3.0-alpha.3+5983e63-dirty kubernetes v1.3.0+507d3a7 features: Basic-Auth Server https://ec2-54-152-68-91.compute-1.amazonaws.com:8443 openshift v1.3.0-alpha.3+5983e63-dirty kubernetes v1.3.0+507d3a7 Test coverage: Test "--record" function with commands "oc scale", "oc edit" and "oc apply". Such as for "oc scale", TCs focus on: 1. Run `oc scale` with "--record=flase", the changes can not be recorded 2. Run `oc scale` with "--record=true", the changes can be recorded 3. Firstly, run `oc scale` with "--record=true", and then run `oc scale` with "--record=flase", only first changes can be recorded 4. Firstly, run `oc scale` with "--record=true", and then run `oc scale` with "--record=true", all changes can be recorded and the first changes was overrided 5. Run `oc scale` without "--record", the changes can not be recorded 6. Firstly, run `oc scale` with "--record=false", and then run `oc scale` without "--record", all changes cannot be recorded 7. Firstly, run `oc scale` with "--record=true", and then run `oc scale` without "--record", all changes can be recorded and the first changes was overrided. 8. Firstly, run `oc scale` with "--record=true", and then run `oc scale` without "--record", and then run `oc scale` without "--record", all changes can be recorded and the first two changes was overrided. 9. Firstly, run `oc scale` with "--record=true", and then run `oc scale` without "--record", and then run `oc scale` with "--record=flase", only the first and second changes can be recorded, and only left the second changes in currently. |