Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for OpenShift routes #93

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTING.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ make test

NOTE: you can adjust the Docker image namespace by overriding the variable `NAMESPACE`, like: `make test NAMESPACE=quay.io/my-username`. The full Docker image name can be customized by overriding `BUILD_IMAGE` instead, like: `make test BUILD_IMAGE=quay.io/my-username/jaeger-operator:0.0.1`

Similar instructions also work for OpenShift, but the target `run-openshift` can be used instead of `run`.

==== Model changes

The Operator SDK generates the `pkg/apis/io/v1alpha1/zz_generated.deepcopy.go` file via the command `make generate`. This should be executed whenever there's a model change (`pkg/apis/io/v1alpha1/types.go`)
Expand Down
10 changes: 10 additions & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,7 @@ required = [
[[constraint]]
name = "github.com/spf13/viper"
version = "1.1.0"

[[constraint]]
name = "github.com/openshift/api"
branch = "release-3.11" # why don't they have tags/versions??
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,14 @@ e2e-tests: cassandra es crd build docker push
@cat deploy/operator.yaml | sed "s~image: jaegertracing\/jaeger-operator\:.*~image: $(BUILD_IMAGE)~gi" >> deploy/test/namespace-manifests.yaml
@go test ./test/e2e/... -kubeconfig $(KUBERNETES_CONFIG) -namespacedMan ../../deploy/test/namespace-manifests.yaml -globalMan ../../deploy/crd.yaml -root .

.PHONY: crd
.PHONY: run
run: crd
@bash -c 'trap "exit 0" INT; OPERATOR_NAME=${OPERATOR_NAME} KUBERNETES_CONFIG=${KUBERNETES_CONFIG} WATCH_NAMESPACE=${WATCH_NAMESPACE} go run -ldflags ${LD_FLAGS} main.go start'

.PHONY: run-openshift
run-openshift: crd
@bash -c 'trap "exit 0" INT; OPERATOR_NAME=${OPERATOR_NAME} KUBERNETES_CONFIG=${KUBERNETES_CONFIG} WATCH_NAMESPACE=${WATCH_NAMESPACE} go run -ldflags ${LD_FLAGS} main.go start --platform=openshift'

.PHONY: es
es:
@kubectl create -f ./test/elasticsearch.yml 2>&1 | grep -v "already exists" || true
Expand Down
15 changes: 5 additions & 10 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ The operator is now ready to create Jaeger instances!

== Installing the operator on OpenShift

The instructions from the previous section also work on OpenShift, but make sure to install the RBAC rules, the CRD and the operator as a privileged user, such as `system:admin`.
The instructions from the previous section also work on OpenShift given that the `operator-openshift.yaml` is used instead of `operator.yaml`. Make sure to install the RBAC rules, the CRD and the operator as a privileged user, such as `system:admin`.

[source,bash]
----
oc login -u system:admin

oc create -f https://raw.githubusercontent.com/jaegertracing/jaeger-operator/master/deploy/rbac.yaml
oc create -f https://raw.githubusercontent.com/jaegertracing/jaeger-operator/master/deploy/crd.yaml
oc create -f https://raw.githubusercontent.com/jaegertracing/jaeger-operator/master/deploy/operator.yaml
oc create -f https://raw.githubusercontent.com/jaegertracing/jaeger-operator/master/deploy/operator-openshift.yaml
----

Once the operator is installed, grant the role `jaeger-operator` to users who should be able to install individual Jaeger instances. The following example creates a role binding allowing the user `developer` to create Jaeger instances:
Expand Down Expand Up @@ -149,18 +149,13 @@ NAME HOSTS ADDRESS PORTS AGE
simplest-query * 192.168.122.34 80 3m
----

IMPORTANT: an `Ingress` object is *not* created when the operator is started with the `--platform=openshift` flag, such as when using the resource `operator-openshift.yaml`.

In this example, the Jaeger UI is available at http://192.168.122.34

=== OpenShift

For OpenShift, the preferred approach is to create a `route` object that will expose the UI under a specific address:

[source,bash]
----
oc create route edge --service simplest-query --port 16686
----

Check the hostname/port with the following command:
When using the `operator-openshift.yaml` resource, the Operator will automatically create a `Route` object for the query services. Check the hostname/port with the following command:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly the route should only be created if ingress.enabled is true? So it has similar behaviour but just has a platform specific outcome.


[source,bash]
----
Expand Down
29 changes: 29 additions & 0 deletions deploy/operator-openshift.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: jaeger-operator
spec:
replicas: 1
selector:
matchLabels:
name: jaeger-operator
template:
metadata:
labels:
name: jaeger-operator
spec:
containers:
- name: jaeger-operator
image: jaegertracing/jaeger-operator:1.7.0
ports:
- containerPort: 60000
name: metrics
args: ["start", "--platform=openshift"]
imagePullPolicy: Always
env:
- name: WATCH_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: OPERATOR_NAME
value: "jaeger-operator"
8 changes: 8 additions & 0 deletions pkg/apis/io/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ import (

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

const (
// FlagPlatformOpenShift represents the value for the 'platform' flag for OpenShift
FlagPlatformOpenShift = "openshift"

// FlagPlatformKubernetes represents the value for the 'platform' flag for Kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: move kubernetes up as the first option - don't want to show favouritism :)

FlagPlatformKubernetes = "kubernetes"
)

// JaegerList is a list of Jaeger structs
type JaegerList struct {
metav1.TypeMeta `json:",inline"`
Expand Down
22 changes: 13 additions & 9 deletions pkg/cmd/start/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (
"syscall"
"time"

"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
stub "github.com/jaegertracing/jaeger-operator/pkg/stub"
"github.com/jaegertracing/jaeger-operator/pkg/version"
sdk "github.com/operator-framework/operator-sdk/pkg/sdk"
k8sutil "github.com/operator-framework/operator-sdk/pkg/util/k8sutil"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/viper"

"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
stub "github.com/jaegertracing/jaeger-operator/pkg/stub"
"github.com/jaegertracing/jaeger-operator/pkg/version"
)

// NewStartCommand starts the Jaeger Operator
Expand All @@ -29,24 +30,27 @@ func NewStartCommand() *cobra.Command {
},
}

cmd.Flags().StringP("jaeger-version", "", version.DefaultJaeger(), "The Jaeger version to use")
cmd.Flags().String("jaeger-version", version.DefaultJaeger(), "The Jaeger version to use")
viper.BindPFlag("jaeger-version", cmd.Flags().Lookup("jaeger-version"))

cmd.Flags().StringP("jaeger-agent-image", "", "jaegertracing/jaeger-agent", "The Docker image for the Jaeger Agent")
cmd.Flags().String("jaeger-agent-image", "jaegertracing/jaeger-agent", "The Docker image for the Jaeger Agent")
viper.BindPFlag("jaeger-agent-image", cmd.Flags().Lookup("jaeger-agent-image"))

cmd.Flags().StringP("jaeger-query-image", "", "jaegertracing/jaeger-query", "The Docker image for the Jaeger Query")
cmd.Flags().String("jaeger-query-image", "jaegertracing/jaeger-query", "The Docker image for the Jaeger Query")
viper.BindPFlag("jaeger-query-image", cmd.Flags().Lookup("jaeger-query-image"))

cmd.Flags().StringP("jaeger-collector-image", "", "jaegertracing/jaeger-collector", "The Docker image for the Jaeger Collector")
cmd.Flags().String("jaeger-collector-image", "jaegertracing/jaeger-collector", "The Docker image for the Jaeger Collector")
viper.BindPFlag("jaeger-collector-image", cmd.Flags().Lookup("jaeger-collector-image"))

cmd.Flags().StringP("jaeger-all-in-one-image", "", "jaegertracing/all-in-one", "The Docker image for the Jaeger all-in-one")
cmd.Flags().String("jaeger-all-in-one-image", "jaegertracing/all-in-one", "The Docker image for the Jaeger all-in-one")
viper.BindPFlag("jaeger-all-in-one-image", cmd.Flags().Lookup("jaeger-all-in-one-image"))

cmd.Flags().StringP("jaeger-cassandra-schema-image", "", "jaegertracing/jaeger-cassandra-schema", "The Docker image for the Jaeger Cassandra Schema")
cmd.Flags().String("jaeger-cassandra-schema-image", "jaegertracing/jaeger-cassandra-schema", "The Docker image for the Jaeger Cassandra Schema")
viper.BindPFlag("jaeger-cassandra-schema-image", cmd.Flags().Lookup("jaeger-cassandra-schema-image"))

cmd.Flags().String("platform", "kubernetes", "The target platform the operator will run. Possible values: 'kubernetes' and 'openshift'")
viper.BindPFlag("platform", cmd.Flags().Lookup("platform"))

return cmd
}

Expand Down
18 changes: 13 additions & 5 deletions pkg/controller/all-in-one.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package controller
import (
"context"

"github.com/jaegertracing/jaeger-operator/pkg/ingress"

"github.com/operator-framework/operator-sdk/pkg/sdk"
"github.com/sirupsen/logrus"
"github.com/spf13/viper"
batchv1 "k8s.io/api/batch/v1"

"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
"github.com/jaegertracing/jaeger-operator/pkg/deployment"
"github.com/jaegertracing/jaeger-operator/pkg/ingress"
"github.com/jaegertracing/jaeger-operator/pkg/route"
"github.com/jaegertracing/jaeger-operator/pkg/storage"
)

Expand Down Expand Up @@ -41,9 +42,16 @@ func (c *allInOneController) Create() []sdk.Object {
os = append(os, svc)
}

qi := ingress.NewQueryIngress(c.jaeger).Get()
if nil != qi {
os = append(os, qi)
if viper.GetString("platform") == v1alpha1.FlagPlatformOpenShift {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth moving this platform specific difference into the pkg/ingress package so reused in production strategy as well - and localises the platform differences?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this kind of logic actually belongs to the controller, as it's the part that decides what should get created (as opposed to the "how", which is inside the specific object packages).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sounds reasonable.

qr := route.NewQueryRoute(c.jaeger).Get()
if nil != qr {
os = append(os, qr)
}
} else {
qi := ingress.NewQueryIngress(c.jaeger).Get()
if nil != qi {
os = append(os, qi)
}
}

return os
Expand Down
27 changes: 23 additions & 4 deletions pkg/controller/all-in-one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ func TestCreateAllInOneDeployment(t *testing.T) {
assertDeploymentsAndServicesForAllInOne(t, name, objs, false)
}

func TestCreateAllInOneDeploymentOnOpenShift(t *testing.T) {
viper.Set("platform", "openshift")
defer viper.Reset()
name := "TestCreateAllInOneDeploymentOnOpenShift"
c := newAllInOneController(context.TODO(), v1alpha1.NewJaeger(name))
objs := c.Create()
assertDeploymentsAndServicesForAllInOne(t, name, objs, false)
}

func TestCreateAllInOneDeploymentWithDaemonSetAgent(t *testing.T) {
name := "TestCreateAllInOneDeploymentWithDaemonSetAgent"

Expand Down Expand Up @@ -71,10 +80,20 @@ func assertDeploymentsAndServicesForAllInOne(t *testing.T, name string, objs []s
fmt.Sprintf("%s-query", name): false,
}

// and the ingress rule
ingresses := map[string]bool{
fmt.Sprintf("%s-query", name): false,
// the ingress rule, if we are not on openshift
var ingresses map[string]bool
var routes map[string]bool
if viper.GetString("platform") == v1alpha1.FlagPlatformOpenShift {
ingresses = map[string]bool{}
routes = map[string]bool{
fmt.Sprintf("%s", name): false,
}
} else {
ingresses = map[string]bool{
fmt.Sprintf("%s-query", name): false,
}
routes = map[string]bool{}
}

assertHasAllObjects(t, name, objs, deployments, daemonsets, services, ingresses)
assertHasAllObjects(t, name, objs, deployments, daemonsets, services, ingresses, routes)
}
9 changes: 8 additions & 1 deletion pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"testing"

osv1 "github.com/openshift/api/route/v1"
"github.com/operator-framework/operator-sdk/pkg/sdk"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -133,7 +134,7 @@ func getDeployments(objs []sdk.Object) []*appsv1.Deployment {
return deps
}

func assertHasAllObjects(t *testing.T, name string, objs []sdk.Object, deployments map[string]bool, daemonsets map[string]bool, services map[string]bool, ingresses map[string]bool) {
func assertHasAllObjects(t *testing.T, name string, objs []sdk.Object, deployments map[string]bool, daemonsets map[string]bool, services map[string]bool, ingresses map[string]bool, routes map[string]bool) {
for _, obj := range objs {
switch typ := obj.(type) {
case *appsv1.Deployment:
Expand All @@ -144,6 +145,8 @@ func assertHasAllObjects(t *testing.T, name string, objs []sdk.Object, deploymen
services[obj.(*v1.Service).Name] = true
case *v1beta1.Ingress:
ingresses[obj.(*v1beta1.Ingress).Name] = true
case *osv1.Route:
routes[obj.(*osv1.Route).Name] = true
default:
assert.Failf(t, "unknown type to be deployed", "%v", typ)
}
Expand All @@ -164,4 +167,8 @@ func assertHasAllObjects(t *testing.T, name string, objs []sdk.Object, deploymen
for k, v := range ingresses {
assert.True(t, v, "Expected %s to have been returned from the list of ingress rules", k)
}

for k, v := range routes {
assert.True(t, v, "Expected %s to have been returned from the list of routes", k)
}
}
15 changes: 12 additions & 3 deletions pkg/controller/production.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (

"github.com/operator-framework/operator-sdk/pkg/sdk"
"github.com/sirupsen/logrus"
"github.com/spf13/viper"
batchv1 "k8s.io/api/batch/v1"

"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
"github.com/jaegertracing/jaeger-operator/pkg/deployment"
"github.com/jaegertracing/jaeger-operator/pkg/ingress"
"github.com/jaegertracing/jaeger-operator/pkg/route"
"github.com/jaegertracing/jaeger-operator/pkg/storage"
)

Expand Down Expand Up @@ -48,9 +50,16 @@ func (c *productionController) Create() []sdk.Object {
components = append(components, svc)
}

qi := ingress.NewQueryIngress(c.jaeger).Get()
if nil != qi {
components = append(components, qi)
if viper.GetString("platform") == v1alpha1.FlagPlatformOpenShift {
qr := route.NewQueryRoute(c.jaeger).Get()
if nil != qr {
components = append(components, qr)
}
} else {
qi := ingress.NewQueryIngress(c.jaeger).Get()
if nil != qi {
components = append(components, qi)
}
}

return components
Expand Down
25 changes: 22 additions & 3 deletions pkg/controller/production_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ func TestCreateProductionDeployment(t *testing.T) {
assertDeploymentsAndServicesForProduction(t, name, objs, false)
}

func TestCreateProductionDeploymentOnOpenShift(t *testing.T) {
viper.Set("platform", "openshift")
defer viper.Reset()
name := "TestCreateProductionDeploymentOnOpenShift"
c := newProductionController(context.TODO(), v1alpha1.NewJaeger(name))
objs := c.Create()
assertDeploymentsAndServicesForProduction(t, name, objs, false)
}

func TestCreateProductionDeploymentWithDaemonSetAgent(t *testing.T) {
name := "TestCreateProductionDeploymentWithDaemonSetAgent"

Expand Down Expand Up @@ -105,9 +114,19 @@ func assertDeploymentsAndServicesForProduction(t *testing.T, name string, objs [
fmt.Sprintf("%s-query", name): false,
}

ingresses := map[string]bool{
fmt.Sprintf("%s-query", name): false,
var ingresses map[string]bool
var routes map[string]bool
if viper.GetString("platform") == v1alpha1.FlagPlatformOpenShift {
ingresses = map[string]bool{}
routes = map[string]bool{
fmt.Sprintf("%s", name): false,
}
} else {
ingresses = map[string]bool{
fmt.Sprintf("%s-query", name): false,
}
routes = map[string]bool{}
}

assertHasAllObjects(t, name, objs, deployments, daemonsets, services, ingresses)
assertHasAllObjects(t, name, objs, deployments, daemonsets, services, ingresses, routes)
}
Loading