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

Make sure --record=false is acknowledged when passed to commands #28234

Merged
merged 1 commit into from
Jul 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions hack/make-rules/test-cmd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,27 @@ runTests() {
# Post-condition: valid-pod is labelled
kube::test::get_object_assert 'pod valid-pod' "{{range$labels_field}}{{.}}:{{end}}" 'valid-pod:new-valid-pod:'

### Record label change
# Pre-condition: valid-pod does not have record annotation
kube::test::get_object_assert 'pod valid-pod' "{{range.items}}{{$annotations_field}}:{{end}}" ''
# Command
kubectl label pods valid-pod record-change=true --record=true "${kube_flags[@]}"
# Post-condition: valid-pod has record annotation
kube::test::get_object_assert 'pod valid-pod' "{{range$annotations_field}}{{.}}:{{end}}" ".*--record=true.*"

### Do not record label change
# Command
kubectl label pods valid-pod no-record-change=true --record=false "${kube_flags[@]}"
# Post-condition: valid-pod's record annotation still contains command with --record=true
kube::test::get_object_assert 'pod valid-pod' "{{range$annotations_field}}{{.}}:{{end}}" ".*--record=true.*"

### Record label change with unspecified flag and previous change already recorded
# Command
kubectl label pods valid-pod new-record-change=true "${kube_flags[@]}"
# Post-condition: valid-pod's record annotation contains new change
kube::test::get_object_assert 'pod valid-pod' "{{range$annotations_field}}{{.}}:{{end}}" ".*new-record-change=true.*"


### Delete POD by label
# Pre-condition: valid-pod POD exists
kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func TestGenerateService(t *testing.T) {
}
cmd := &cobra.Command{}
cmd.Flags().Bool(cmdutil.ApplyAnnotationsFlag, false, "")
cmd.Flags().Bool("record", false, "Record current kubectl command in the resource annotation.")
cmd.Flags().Bool("record", false, "Record current kubectl command in the resource annotation. If set to false, do not record the command. If set to true, record the command. If not set, default to updating the existing annotation value only if one already exists.")
cmdutil.AddPrinterFlags(cmd)
cmdutil.AddInclude3rdPartyFlags(cmd)
addRunFlags(cmd)
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/util/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func UpdateObject(info *resource.Info, codec runtime.Codec, updateFn func(runtim

// AddCmdRecordFlag adds --record flag to command
func AddRecordFlag(cmd *cobra.Command) {
cmd.Flags().Bool("record", false, "Record current kubectl command in the resource annotation.")
cmd.Flags().Bool("record", false, "Record current kubectl command in the resource annotation. If set to false, do not record the command. If set to true, record the command. If not set, default to updating the existing annotation value only if one already exists.")
}

func GetRecordFlag(cmd *cobra.Command) bool {
Expand Down Expand Up @@ -489,7 +489,7 @@ func ContainsChangeCause(info *resource.Info) bool {

// ShouldRecord checks if we should record current change cause
func ShouldRecord(cmd *cobra.Command, info *resource.Info) bool {
return GetRecordFlag(cmd) || ContainsChangeCause(info)
return GetRecordFlag(cmd) || (ContainsChangeCause(info) && !cmd.Flags().Changed("record"))
Copy link
Member

Choose a reason for hiding this comment

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

Please update the flag documentation to be more clear. Also please add a test for this.

}

// GetThirdPartyGroupVersions returns the thirdparty "group/versions"s and
Expand Down