Add ocdev catalog command #243
Conversation
concaf
added
the
do-not-merge/work-in-progress
|
@kadel would love your initial feedback |
pkg/occlient/occlient.go
Outdated
| return GetImageStreams(openShiftNameSpace) | ||
| } | ||
|
|
||
| func GetImageStreams(namespace string) ([]string, error) { |
There was a problem hiding this comment.
It might be better to call this function GetImageStreamNames if it is returning just names.
Also comment would be nice ;-)
pkg/occlient/occlient.go
Outdated
| @@ -380,10 +385,25 @@ func getExposedPorts(image *imagev1.ImageStreamImage) ([]corev1.ContainerPort, e | |||
| return ports, nil | |||
| } | |||
|
|
|||
| func GetDefaultBuilderImages() ([]string, error) { | |||
pkg/occlient/occlient.go
Outdated
| @@ -49,6 +52,41 @@ var ( | |||
| namespace string | |||
| ) | |||
|
|
|||
| func init() { | |||
There was a problem hiding this comment.
We shouldn't use init for this :-(
We will have to construct struct that will represent openshift client and that will hold all the client-go clients
|
a few pointers, but in general I think goes in the right direction |
|
@kadel made the changes, also added the service catalog support, PTAL! |
|
|
||
| // clusterServiceClassExists returns true if the given external name of the | ||
| // cluster service class exists in the cluster, and false otherwise | ||
| func (c *Client) clusterServiceClassExists(name string) bool { |
There was a problem hiding this comment.
we're not using this in this PR, but we will need this when we implement deploying using s2i with this, so maybe we can keep this util function here
|
|
||
| // imageStreamExists returns true if the given image stream exists in the given | ||
| // namespace | ||
| func (c *Client) imageStreamExists(name string, namespace string) bool { |
There was a problem hiding this comment.
we're not using this in this PR, but we will need this when we implement deploying using s2i with this, so maybe we can keep this util function here
pkg/occlient/occlient.go
Outdated
| @@ -34,18 +35,23 @@ import ( | |||
| "github.com/openshift/source-to-image/pkg/tar" | |||
| s2ifs "github.com/openshift/source-to-image/pkg/util/fs" | |||
|
|
|||
| "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" | |||
There was a problem hiding this comment.
This should be named properly. v1beta1 is too generic and it will collide with other objects.
How about catalogv1beta1 "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" so it follows naming convention with other apis that we are importing??
There was a problem hiding this comment.
changed to scv1beta1
concaf
removed
the
do-not-merge/work-in-progress
|
We have to somehow display that this requires service catalog. Probably in the readme and also in help messages. Another thing is that it displays a lot of stuff that currently can't be deployed (everything from service catalog). We should hide it from output until |
|
@kadel that means hiding the entire service catalog output |
cmd/catalog.go
Outdated
| } | ||
|
|
||
| var catalogSearchCmd = &cobra.Command{ | ||
| Use: "search", |
There was a problem hiding this comment.
search <search_term> or something like this to indicate that there is one argument required and what that argument it
pkg/catalog/catalog.go
Outdated
| ) | ||
|
|
||
| // List lists all the available component types | ||
| // Currently queries the image streams in openshift namespace |
There was a problem hiding this comment.
Is that comment true. I can see that in code GetClusterServiceClassExternalNames() is called
|
@kadel commented out service catalog for now, and made the changes, PTAL |
Yes, I doesn't make sense to show it right now when we don't have a way to deploy those thinks |
|
It would be nice to clean up Cobra metadata, so we have nice command line output with examples. Otherwise this LGTM |
|
@kadel added more description to the catalog commands, PTAL |
This commit adds github.com/kubernetes-incubator/service-catalog, version: v0.1.9 and github.com/satori/go.uuid, version: v1.1.0 ( which is required by service-catalog) to the vendor directory. The standard procedure of adding the dependencies to glide.yaml and then running `glide update --strip-vendor` was followed.
cmd/catalog.go
Outdated
| # Get the supported components | ||
| ocdev catalog list | ||
|
|
||
| # Deploy the component |
There was a problem hiding this comment.
There should be only examples for this command
cmd/catalog.go
Outdated
| Example: `# Search for a component | ||
| ocdev catalog search pyt | ||
|
|
||
| # Deploy the component |
This commit adds support for `ocdev catalog` command which currently has 2 commands, list and search, to list all the components and to search for a component. To achieve this, the service catalog API has been used.
Fix #194 #35