Skip to content

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Jun 1, 2022

What does this PR do?

When listing configmaps and secrets on the cluster, the API can return objects in a random order (for two configmaps, I see the reverse order almost exactly 1/8th of the time, at random)

This can result in the operator defining a Deployment spec for workspaces with the envFrom fields switched, if those configmaps are automounted as environment variables. To get around this, we ignore the order of envFrom entries when deciding if the deployment needs to be updated.

What issues does this PR fix or reference?

Closes #849

Is it tested? How?

Create two mount-as: env configmaps in a namespace and attempt to start a workspace. Since the issue does not reproduce consistently, the only way to be 100% sure is to add debug logging in pkg/provision/automount/configmap.go to list the order of configmaps seen -- add the following to line 39 in that file

	fmt.Printf("configmaps_found:")
	for _, configmap := range configmaps.Items {
		fmt.Printf("%s, ", configmap.Name)
	}
	fmt.Printf("\n")

and then grep the DWO logs for "configmaps_found". You should see different orders appear:

configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:che-proxy-settings, my-settings, 
configmaps_found:my-settings, che-proxy-settings, 
configmaps_found:che-proxy-settings, my-settings,

You might have to repeatedly edit a workspace to get enough reconciles to see the bug:

for i in {1..100}; do
oc patch dw theia-next --type merge -p "
metadata:
  annotations:
    touch: \"$i\"
"
sleep 0.5
done

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

When listing configmaps and secrets on the cluster, the API can return
objects in a random order (for two configmaps, I see the reverse order
almost exactly 1/8th of the time)

This can result in the operator defining a Deployment spec for
workspaces with the envFrom fields switched, if those configmaps are
automounted as environment variables. To get around this, we ignore the
order of envFrom entries when deciding if the deployment needs to be
updated.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@amisevsk amisevsk requested a review from ibuziuk as a code owner June 1, 2022 20:24
@openshift-ci openshift-ci bot added the approved label Jun 1, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jun 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, ibuziuk

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

@ibuziuk ibuziuk merged commit 4a3f1a1 into devfile:main Jun 2, 2022
@amisevsk amisevsk deleted the ignore-envfrom-order branch June 2, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automount envFrom fields can appear in random order

2 participants