WRKLDS-1676: OCM config: Remove obsolete fields #2364
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@tchap: This pull request references WRKLDS-1676 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hello @tchap! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/M
|
@tchap: This pull request references WRKLDS-1676 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/hold I actually need to check more thoroughly where the struct is being used and whether the fields are really unused. |
openshift-ci
bot
added
the
do-not-merge/hold
As OCM moves to use library-go for configuration, some fields present into OpenShiftControllerManagerConfig are no longer needed since they are actually accessed via GenericOperatorConfig. Leaving the fields there commented out as a tombstone.
tchap
force-pushed
the
openshiftcontrollermanager-obsolete-fields
branch
from
8dafc05 to
f5a5920
Compare
openshift-ci
bot
added
size/S
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
| @@ -258,7 +258,10 @@ const ( | |||
| type OpenShiftControllerManagerConfig struct { | |||
| metav1.TypeMeta `json:",inline"` | |||
|
|
|||
| KubeClientConfig configv1.KubeClientConfig `json:"kubeClientConfig"` | |||
| // KubeClientConfig is no longer being used. | |||
There was a problem hiding this comment.
No longer? Or never? Was it ever actually implemented?
There was a problem hiding this comment.
It was. Before using library-go to setup the executable, OCM used this field to configure the client. Kubeconfig file path is now being passed in using a command line flag and library-go machinery uses that to init a client.
There was a problem hiding this comment.
And who typically writes to this object? Is this generally only written to by an operator?
There was a problem hiding this comment.
Yeah, it's used by OCMO or Hypershift. But neither accesses this field.
Hypershift only sets it statically in a YAML and it's been updated for the library-go way of config in openshift/hypershift#6203
OCMO doesn't set the field, because OCM uses in-cluster config there and an explicit kubeconfig is not needed.
|
@tchap: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, tchap 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 |
openshift-ci
bot
added
the
approved
|
[ART PR BUILD NOTIFIER] Distgit: ose-cluster-config-api |
As OCM moves to use library-go for configuration, some fields present into OpenShiftControllerManagerConfig are no longer needed since they are actually accessed via GenericOperatorConfig.
There is no stability promise tied to these fields, they are only being used internally by OCM.
Leaving the fields there commented out as a tombstone.