WIP Subscription CatalogSource Status #818
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njhale 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-robot
added
approved
| var _ references.ObjectReferencerFunc = ObjectReferenceFor | ||
|
|
||
| // ObjectReferenceFor generates an ObjectReference for the given resource if it's provided by the operators.coreos.com API group. | ||
| func ObjectReferenceFor(obj interface{}) (*corev1.ObjectReference, error) { |
There was a problem hiding this comment.
Is there a reason we're not just using the utils here? https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/tools/reference/ref.go
There was a problem hiding this comment.
I had assumed a util like this existed, but my searching over kubernetes/kubernetes didn't pick it up. Is there a canonical reference for useful kube utils that I'm missing???
| } | ||
|
|
||
| // Zero-out so started will be GC'd since this goroutine will live for a while | ||
| started = nil |
There was a problem hiding this comment.
alternative would be to put this in a scoped block, e.g:
o.Log.Info("starting informers...")
{
started := make(map[cache.SharedIndexInformer]struct{})
// start informers
}
o.Log.Info()nbd though
| // or the status has changed and returns true if the status was set; false otherwise. | ||
| func (status *SubscriptionStatus) SetSubscriptionCatalogStatus(catalogStatus SubscriptionCatalogStatus) bool { | ||
| target := catalogStatus.CatalogSourceRef | ||
| if target == nil && target.APIVersion == SchemeGroupVersion.String() && target.Kind == SubscriptionKind { |
There was a problem hiding this comment.
why are we checking the kind of the catalogsourceref? shouldn't it always be catalogsource?
or rather, shouldn't this check for kind != CatalogSourceKind?
| op.subQueue = subscriptionQueue | ||
| for _, informer := range subscriptionQueueInformers { | ||
|
|
||
| // Register the QueueInformer for SubscriptionStatus.CatalogStatus |
There was a problem hiding this comment.
nit - this comment doesn't seem right
| // NewFakeOperator creates a new operator using fake clients | ||
| func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extObjs []runtime.Object, regObjs []runtime.Object, namespace string, stopCh <-chan struct{}) (*Operator, []cache.InformerSynced, error) { | ||
| // NewFakeOperator creates a new operator using fake clients. | ||
| func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extObjs []runtime.Object, regObjs []runtime.Object, operatorNamespace string, namespaces []string, stopCh <-chan struct{}) (*Operator, error) { |
There was a problem hiding this comment.
the namespace to operatorNamespace change is confusing - in other parts of the tests we use namespace to refer to the namespace that OLM is running in and operatorNamespace to refer to the namespace that OLM is watching.
i.e. if we change it here we should change it everywhere to be consistent
- Add ObjectReferencer interface - Add ObjectReferencer for OLM types - Remove cyclic dependencies between OLM type packages
njhale
force-pushed
the
sub-catsrc-status
branch
from
8f1a146 to
5afe09b
Compare
openshift-ci-robot
added
size/L
njhale
force-pushed
the
sub-catsrc-status
branch
from
5afe09b to
70cc503
Compare
|
@njhale: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
|
Closing out in favor of starting with #820 |
Part 1 of 2 in a pair of PRs aimed at surfacing better Subscription status.