Drop code that expects the config map in openshift-config namespace to have policy-configmap name #268
Conversation
To avoid tests passing due to the same name for the config map. User may use a different name. Though, since the namespaces are different, use different name to properly test the config map from openshift-config namespace was properly copy-pasted.
…o have policy-configmap name
|
@damemi FYI |
openshift-ci-robot
added
the
approved
| resourcesynccontroller.ResourceLocation{Namespace: operatorclient.TargetNamespace, Name: scheduler.Spec.Policy.Name}, | ||
| resourcesynccontroller.ResourceLocation{Namespace: operatorclient.GlobalUserSpecifiedConfigNamespace, Name: "policy-configmap"}); err != nil { |
There was a problem hiding this comment.
I agree these lines are wrong (namespace/name are switched) but do we want to remove them?
Target config controller does basically the same copy, but only when the scheduler CR is updated. Do we have a test for when the user creates a custom policy configmap (in openshift-config), adds it to the scheduler CR, then later updates that custom policy configmap directly?
I am wondering if, without this, changes will only be synced between openshift-config and the target namespace when the scheduler CR is updated (so really only by changing the custom policy configmap name)
There was a problem hiding this comment.
Target config controller does basically the same copy, but only when the scheduler CR is updated.
Are you sure about this? I tested it locally and the configmap got synced after I edited openshift-config one even without the lines.
There was a problem hiding this comment.
I actually wasn't sure, sorry, meant to pose that as more "did we check this?" based on talking about it offline the other day. If it works then cool! But do we have a test for that case?
There was a problem hiding this comment.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damemi, ingvagabund 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 |
Copy pasting
openshift-config/CMNametoopenshift-kube-scheduler/policy-configmapis already carried out in the config observer code. Also, resource sync controller only copy pastes the user specified config when it'spolicy-configmapnamed (SyncConfigMaphas the first argument as a destination. the second one as a source).