Merge pull request #709 from njhale/grpc-address

feat(catalogsource): allow grpc source types that don't require an image

Author: openshift-merge-robot

cmd/catalog/main.go

@@ -99,4 +99,3 @@ func main() {
 	_, done := catalogOperator.Run(stopCh)
 	<-done
 }
-

deploy/chart/templates/0000_50_olm_05-catalogsource.crd.yaml

@@ -62,9 +62,13 @@ spec:
               type: string
               description: The name of a ConfigMap that holds the entries for an in-memory catalog.
 
+            address:
+              type: string
+              description: An optional address. When set, directs OLM to connect to use a pre-existing registry server at this address.
+
             image:
               type: string
-              description: An image that serves a grpc registry. Only valid for `grpc` sourceType.
+              description: An image that serves a grpc registry. Only valid for `grpc` sourceType. If both image and address are set, OLM does not use the address field.
 
             displayName:
               type: string

deploy/ocp/manifests/0.8.1/0000_50_olm_05-catalogsource.crd.yaml

@@ -64,9 +64,13 @@ spec:
               type: string
               description: The name of a ConfigMap that holds the entries for an in-memory catalog.
 
+            address:
+              type: string
+              description: An optional address. When set, directs OLM to connect to use a pre-existing registry server at this address.
+
             image:
               type: string
-              description: An image that serves a grpc registry. Only valid for `grpc` sourceType.
+              description: An image that serves a grpc registry. Only valid for `grpc` sourceType. If both image and address are set, OLM does not use the address field.
 
             displayName:
               type: string

deploy/okd/manifests/0.8.1/0000_50_olm_05-catalogsource.crd.yaml

@@ -64,9 +64,13 @@ spec:
               type: string
               description: The name of a ConfigMap that holds the entries for an in-memory catalog.
 
+            address:
+              type: string
+              description: An optional address. When set, directs OLM to connect to use a pre-existing registry server at this address.
+
             image:
               type: string
-              description: An image that serves a grpc registry. Only valid for `grpc` sourceType.
+              description: An image that serves a grpc registry. Only valid for `grpc` sourceType. If both image and address are set, OLM does not use the address field.
 
             displayName:
               type: string

deploy/upstream/manifests/0.8.1/0000_50_olm_05-catalogsource.crd.yaml

@@ -64,9 +64,13 @@ spec:
               type: string
               description: The name of a ConfigMap that holds the entries for an in-memory catalog.
 
+            address:
+              type: string
+              description: An optional address. When set, directs OLM to connect to use a pre-existing registry server at this address.
+
             image:
               type: string
-              description: An image that serves a grpc registry. Only valid for `grpc` sourceType.
+              description: An image that serves a grpc registry. Only valid for `grpc` sourceType. If both image and address are set, OLM does not use the address field.
 
             displayName:
               type: string

go.sum

@@ -297,6 +297,7 @@ k8s.io/apimachinery v0.0.0-20181203235515-3d8ee2261517/go.mod h1:ccL7Eh7zubPUSh9
 k8s.io/apimachinery v0.0.0-20190118094746-1525e4dadd2d/go.mod h1:ccL7Eh7zubPUSh9A3USN90/OzHNSVN6zxzde07TDCL0=
 k8s.io/apimachinery v0.0.0-20190208202428-1a579f8a7b42 h1:ju2lLx7i6XE8A9QLtwxlQngelP8SosMIjK4IYE/TLFI=
 k8s.io/apimachinery v0.0.0-20190208202428-1a579f8a7b42/go.mod h1:ccL7Eh7zubPUSh9A3USN90/OzHNSVN6zxzde07TDCL0=
+k8s.io/apimachinery v0.0.0-20190213190008-5eb658cbe970 h1:alS6VCwjuZxqOAhOnpSY/TbMvwVRAOWSTeE63v5Iz+M=
 k8s.io/apiserver v0.0.0-20181026151315-13cfe3978170 h1:CqI85nZvPaV+7JFono0nAOGOx2brocqefcOhDPVhHKI=
 k8s.io/apiserver v0.0.0-20181026151315-13cfe3978170/go.mod h1:6bqaTSOSJavUIXUtfaR9Os9JtTCm8ZqH2SUl2S60C4w=
 k8s.io/client-go v0.0.0-20180718001006-59698c7d9724/go.mod h1:7vJpHMYJwNQCWgzmNV+VYUl1zCObLyodBc8nIyt8L5s=

manifests/0000_50_olm_05-catalogsource.crd.yaml

@@ -62,9 +62,13 @@ spec:
               type: string
               description: The name of a ConfigMap that holds the entries for an in-memory catalog.
 
+            address:
+              type: string
+              description: An optional address. When set, directs OLM to connect to use a pre-existing registry server at this address.
+
             image:
               type: string
-              description: An image that serves a grpc registry. Only valid for `grpc` sourceType.
+              description: An image that serves a grpc registry. Only valid for `grpc` sourceType. If both image and address are set, OLM does not use the address field.
 
             displayName:
               type: string

pkg/api/apis/operators/v1alpha1/catalogsource_types.go

@@ -18,16 +18,43 @@ const (
 type SourceType string
 
 const (
-	SourceTypeInternal  SourceType = "internal"
+	// SourceTypeInternal (deprecated) specifies a CatalogSource of type SourceTypeConfigmap
+	SourceTypeInternal SourceType = "internal"
+
+	// SourceTypeConfigmap specifies a CatalogSource that generates a configmap-server registry
 	SourceTypeConfigmap SourceType = "configmap"
-	SourceTypeGrpc      SourceType = "grpc"
+
+	// SourceTypeGrpc specifies a CatalogSource that can use an operator registry image to generate a
+	// registry-server or connect to a pre-existing registry at an address.
+	SourceTypeGrpc SourceType = "grpc"
 )
 
 type CatalogSourceSpec struct {
+	// SourceType is the type of source
 	SourceType SourceType `json:"sourceType"`
-	ConfigMap  string     `json:"configMap,omitempty"`
-	Image      string     `json:"image,omitempty"`
-	Secrets    []string   `json:"secrets,omitempty"`
+
+	// ConfigMap is the name of the ConfigMap to be used to back a configmap-server registry.
+	// Only used when SourceType = SourceTypeConfigmap or SourceTypeInternal.
+	// +Optional
+	ConfigMap string `json:"configMap,omitempty"`
+
+	// Address is a host that OLM can use to connect to a pre-existing registry.
+	// Format: <registry-host or ip>:<port>
+	// Only used when SourceType = SourceTypeGrpc.
+	// Ignored when the Image field is set.
+	// +Optional
+	Address string `json:"address,omitempty"`
+
+	// Image is an operator-registry container image to instantiate a registry-server with.
+	// Only used when SourceType = SourceTypeGrpc.
+	// If present, the address field is ignored.
+	// +Optional
+	Image string `json:"image,omitempty"`
+
+	// Secrets represent set of secrets that can be used to access the contents of the catalog.
+	// It is best to keep this list small, since each will need to be tried for every catalog entry.
+	// +Optional
+	Secrets []string `json:"secrets,omitempty"`
 
 	// Metadata
 	DisplayName string `json:"displayName,omitempty"`
@@ -53,6 +80,7 @@ type CatalogSourceStatus struct {
 	RegistryServiceStatus *RegistryServiceStatus      `json:"registryService,omitempty"`
 	LastSync              metav1.Time                 `json:"lastSync,omitempty"`
 }
+
 type ConfigMapResourceReference struct {
 	Name      string `json:"name"`
 	Namespace string `json:"namespace"`
@@ -71,6 +99,13 @@ type CatalogSource struct {
 	Status CatalogSourceStatus `json:"status"`
 }
 
+func (c *CatalogSource) Address() string {
+	if c.Spec.Address != "" {
+		return c.Spec.Address
+	}
+	return c.Status.RegistryServiceStatus.Address()
+}
+
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 type CatalogSourceList struct {
 	metav1.TypeMeta `json:",inline"`

pkg/controller/operators/catalog/operator.go

@@ -77,7 +77,7 @@ func NewOperator(kubeconfigPath string, logger *logrus.Logger, wakeupInterval ti
 		watchedNamespaces = []string{metav1.NamespaceAll}
 	}
 
-	// Create a new client for ALM types (CRs)
+	// Create a new client for OLM types (CRs)
 	crClient, err := client.NewClient(kubeconfigPath)
 	if err != nil {
 		return nil, err
@@ -381,24 +381,26 @@ func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) {
 
 			return nil
 		}
+		
+		logger.Debug("catsrc configmap state good, checking registry pod")
 	}
 
-	reconciler := o.reconciler.ReconcilerForSourceType(catsrc.Spec.SourceType)
+	reconciler := o.reconciler.ReconcilerForSource(catsrc)
 	if reconciler == nil {
+		// TODO: Add failure status on catalogsource and remove from sources
 		return fmt.Errorf("no reconciler for source type %s", catsrc.Spec.SourceType)
 	}
 
-	logger.Debug("catsrc configmap state good, checking registry pod")
 
 	// if registry pod hasn't been created or hasn't been updated since the last configmap update, recreate it
 	if catsrc.Status.RegistryServiceStatus == nil || catsrc.Status.RegistryServiceStatus.CreatedAt.Before(&catsrc.Status.LastSync) {
-		logger.Debug("registry pod scheduled for recheck")
+		logger.Debug("registry server scheduled recheck")
 
 		if err := reconciler.EnsureRegistryServer(out); err != nil {
 			logger.WithError(err).Warn("couldn't ensure registry server")
 			return err
 		}
-		logger.Debug("ensured registry pod")
+		logger.Debug("ensured registry server")
 
 		out.Status.RegistryServiceStatus.CreatedAt = timeNow()
 		out.Status.LastSync = timeNow()
@@ -410,18 +412,18 @@ func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) {
 		}
 
 		o.sourcesLastUpdate = timeNow()
-		logger.Debug("registry pod recreated")
+		logger.Debug("registry server recreated")
 
 		return nil
 	}
-	logger.Debug("registry pod state good")
+	logger.Debug("registry state good")
 
 	// update operator's view of sources
 	sourcesUpdated := false
 	func() {
 		o.sourcesLock.Lock()
 		defer o.sourcesLock.Unlock()
-		address := catsrc.Status.RegistryServiceStatus.Address()
+		address := catsrc.Address()
 		currentSource, ok := o.sources[sourceKey]
 		logger = logger.WithField("currentSource", sourceKey)
 		if !ok || currentSource.Address != address || catsrc.Status.LastSync.After(currentSource.LastConnect.Time) {
@@ -664,7 +666,7 @@ func (o *Operator) ensureResolverSources(logger *logrus.Entry, namespace string)
 		logger.WithField("clientState", client.Conn.GetState()).Debug("source")
 		if client.Conn.GetState() == connectivity.TransientFailure {
 			logger.WithField("clientState", client.Conn.GetState()).Debug("waiting for connection")
-			ctx, _ := context.WithTimeout(context.TODO(), 5*time.Second)
+			ctx, _ := context.WithTimeout(context.TODO(), 2*time.Second)
 			changed := client.Conn.WaitForStateChange(ctx, connectivity.TransientFailure)
 			if !changed {
 				logger.WithField("clientState", client.Conn.GetState()).Debug("source in transient failure and didn't recover")

pkg/controller/operators/catalog/subscriptions_test.go

@@ -499,7 +499,7 @@ func TestSyncSubscriptions(t *testing.T) {
 			require.NoError(t, err)
 
 			o.reconciler = &fakes.FakeReconcilerFactory{
-				ReconcilerForSourceTypeStub: func(sourceType v1alpha1.SourceType) reconciler.RegistryReconciler {
+				ReconcilerForSourceStub: func(source *v1alpha1.CatalogSource) reconciler.RegistryReconciler {
 					return &fakes.FakeRegistryReconciler{
 						EnsureRegistryServerStub: func(source *v1alpha1.CatalogSource) error {
 							return nil