Skip to content

Conversation

@abhinavdahiya
Copy link
Contributor

These functions currently exist in https://github.com/openshift/cluster-version-operator/blob/0eaaaa01e881933223f5afcb57de73dbaf793b54/lib/resourcemerge/os.go#L36-L109.
This removes dependency of operators from cvo completely.

@deads2k hopefully this import github.com/library-go/pkg/config/clusteroperator/v1 is sane for getting these functions on ClusterOperatorStatus.

/cc @smarterclayton @deads2k

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 19, 2018
@abhinavdahiya
Copy link
Contributor Author

ping :)

@deads2k
Copy link
Contributor

deads2k commented Nov 26, 2018

You sure want this or want to simply update the ClusterOperator resource to use the operator/v1 conditions?

@abhinavdahiya
Copy link
Contributor Author

You sure want this or want to simply update the ClusterOperator resource to use the operator/v1 conditions?

Either is fine. I don't have any strong prefs on that, whatever you think is more correct.
/cc @smarterclayton

@deads2k
Copy link
Contributor

deads2k commented Nov 27, 2018

Either is fine. I don't have any strong prefs on that, whatever you think is more correct.
/cc @smarterclayton

I think we should collapse our status conditions into a single type. It's a quick change in openshift/api and doesn't affect serialization in any serious way.

@abhinavdahiya
Copy link
Contributor Author

@deads2k
https://github.com/openshift/api/blob/master/operator/v1/register.go#L13 prevents us from using operatorv1.OperatorCondition in configv1.ClusterOperator.Status.Conditions (import cycle)

Is other way around ie moving operatorv1.OperatorCondition to configv1 acceptable?

@deads2k
Copy link
Contributor

deads2k commented Nov 27, 2018

@deads2k
https://github.com/openshift/api/blob/master/operator/v1/register.go#L13 prevents us from using operatorv1.OperatorCondition in configv1.ClusterOperator.Status.Conditions (import cycle)

Is other way around ie moving operatorv1.OperatorCondition to configv1 acceptable?

I won't hold this up on that. I'm personally disinclined to move it in that direction.

/lgtm
/hold

holding for nits (at least the order). Feel free to self-tag after that.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Nov 27, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, deads2k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2018
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 27, 2018
@abhinavdahiya
Copy link
Contributor Author

Feel free to self-tag after that.

Adding the lgtm label manually as @deads2k allowed me to do it. 😇

@abhinavdahiya abhinavdahiya added the lgtm Indicates that a PR is ready to be merged. label Nov 27, 2018
@deads2k deads2k removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2018
@openshift-merge-robot openshift-merge-robot merged commit f0d14bd into openshift:master Nov 28, 2018
@abhinavdahiya abhinavdahiya deleted the config_v1_helpers branch January 3, 2019 23:43
bertinatto pushed a commit to bertinatto/library-go that referenced this pull request Jul 2, 2020
config: add status helper funcs for v1.ClusterOperator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants