-
Notifications
You must be signed in to change notification settings - Fork 345
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 link to openshift console #1142
Changes from 9 commits
5e0607e
418f6ff
b1ae803
0c7ea2b
f62ed66
1d8121d
d266ad8
a7f7a61
773576e
6a4a3df
04e2ac5
7eba146
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package consolelink | ||
|
||
import ( | ||
"fmt" | ||
|
||
consolev1 "github.com/openshift/api/console/v1" | ||
routev1 "github.com/openshift/api/route/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
||
v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" | ||
) | ||
|
||
// RouteAnnotation used to annotate the link with the route name | ||
var RouteAnnotation = "consolelink.jaegertracing.io/route" | ||
|
||
// Get returns an ConsoleLink specification for the current instance | ||
func Get(jaeger *v1.Jaeger, route *routev1.Route) *consolev1.ConsoleLink { | ||
// If ingress is not enable there is no reason for create a console link | ||
if jaeger.Spec.Ingress.Enabled != nil && *jaeger.Spec.Ingress.Enabled == false { | ||
return nil | ||
} | ||
|
||
return &consolev1.ConsoleLink{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: jaeger.Namespace + ".jaeger-consolelink-" + jaeger.Name, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This name could be improved: this is a I'd say that you can just use the $ kubectl get routes -n observability
NAME HOST/PORT PATH SERVICES PORT TERMINATION WILDCARD
simplest simplest-observability.apps-crc.testing simplest-query <all> reencrypt None For reference, here's how the some built-in console links look like: $ kubectl get consolelinks --all-namespaces
NAME TEXT URL MENU AGE
observability.jaeger-consolelink-simplest Jaeger [simplest] https://simplest-observability.apps-crc.testing 8m14s
openshift-blog OpenShift Blog https://blog.openshift.com 19d
openshift-learning-portal Learning Portal https://learn.openshift.com/?ref=webconsole 19d |
||
Namespace: jaeger.Namespace, | ||
Labels: map[string]string{ | ||
"app.kubernetes.io/instance": jaeger.Name, | ||
"app.kubernetes.io/managed-by": "jaeger-operator", | ||
}, | ||
Annotations: map[string]string{ | ||
RouteAnnotation: route.Name, | ||
}, | ||
}, | ||
Spec: consolev1.ConsoleLinkSpec{ | ||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Location: consolev1.NamespaceDashboard, | ||
Link: consolev1.Link{ | ||
Text: "Jaeger [" + jaeger.Name + "]", | ||
}, | ||
NamespaceDashboard: &consolev1.NamespaceDashboardSpec{ | ||
Namespaces: []string{ | ||
jaeger.Namespace, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
// UpdateHref returns an ConsoleLink with the href value derived from the route | ||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func UpdateHref(routes []routev1.Route, links []consolev1.ConsoleLink) []consolev1.ConsoleLink { | ||
var updated []consolev1.ConsoleLink | ||
mapRoutes := make(map[string]string) | ||
for _, route := range routes { | ||
mapRoutes[route.Name] = route.Spec.Host | ||
} | ||
for _, cl := range links { | ||
routeName := cl.Annotations[RouteAnnotation] | ||
// Only append it if we can found the route | ||
if host, ok := mapRoutes[routeName]; ok { | ||
cl.Spec.Href = fmt.Sprintf("https://%s", host) | ||
updated = append(updated, cl) | ||
} | ||
//TODO: log if not found the route associated with the link | ||
} | ||
return updated | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||
package consolelink | ||||||
|
||||||
import ( | ||||||
"testing" | ||||||
|
||||||
consolev1 "github.com/openshift/api/console/v1" | ||||||
corev1 "github.com/openshift/api/route/v1" | ||||||
"github.com/stretchr/testify/assert" | ||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
"k8s.io/apimachinery/pkg/types" | ||||||
|
||||||
v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" | ||||||
) | ||||||
|
||||||
func TestConsoleLinkGet(t *testing.T) { | ||||||
jaegerName := "TestConsoleLinkJaeger" | ||||||
jaegerNamespace := "TestNS" | ||||||
routerNamer := "TestConsoleLinkRoute" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
jaeger := v1.NewJaeger(types.NamespacedName{Name: jaegerName, Namespace: jaegerNamespace}) | ||||||
route := &corev1.Route{ | ||||||
TypeMeta: metav1.TypeMeta{ | ||||||
Kind: "Route", | ||||||
APIVersion: "route.openshift.io/v1", | ||||||
}, | ||||||
ObjectMeta: metav1.ObjectMeta{ | ||||||
Name: routerNamer, | ||||||
}, | ||||||
} | ||||||
|
||||||
link := Get(jaeger, route) | ||||||
assert.Equal(t, jaeger.Namespace+".jaeger-consolelink-"+jaeger.Name, link.Name) | ||||||
assert.Contains(t, link.Annotations, RouteAnnotation) | ||||||
assert.Equal(t, routerNamer, link.Annotations[RouteAnnotation]) | ||||||
} | ||||||
|
||||||
func TestUpdateHref(t *testing.T) { | ||||||
jaegerName := "TestConsoleLinkJaeger" | ||||||
jaegerNamespace := "TestNS" | ||||||
routerNamer := "TestConsoleLinkRoute" | ||||||
jaeger := v1.NewJaeger(types.NamespacedName{Name: jaegerName, Namespace: jaegerNamespace}) | ||||||
route := corev1.Route{ | ||||||
TypeMeta: metav1.TypeMeta{ | ||||||
Kind: "Route", | ||||||
APIVersion: "route.openshift.io/v1", | ||||||
}, | ||||||
ObjectMeta: metav1.ObjectMeta{ | ||||||
Name: routerNamer, | ||||||
}, | ||||||
} | ||||||
|
||||||
link := Get(jaeger, &route) | ||||||
assert.Equal(t, link.Spec.Href, "") | ||||||
route.Spec.Host = "namespace.somehostname" | ||||||
newLinks := UpdateHref([]corev1.Route{route}, []consolev1.ConsoleLink{*link}) | ||||||
assert.Equal(t, "https://"+route.Spec.Host, newLinks[0].Spec.Href) | ||||||
|
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package jaeger | ||
|
||
import ( | ||
"context" | ||
|
||
osconsolev1 "github.com/openshift/api/console/v1" | ||
log "github.com/sirupsen/logrus" | ||
"go.opentelemetry.io/otel/global" | ||
"sigs.k8s.io/controller-runtime/pkg/client" | ||
|
||
v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" | ||
"github.com/jaegertracing/jaeger-operator/pkg/inventory" | ||
"github.com/jaegertracing/jaeger-operator/pkg/tracing" | ||
) | ||
|
||
func (r *ReconcileJaeger) applyConsoleLinks(ctx context.Context, jaeger v1.Jaeger, desired []osconsolev1.ConsoleLink) error { | ||
tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer) | ||
ctx, span := tracer.Start(ctx, "applyConsoleLinks") | ||
defer span.End() | ||
|
||
opts := []client.ListOption{ | ||
client.InNamespace(jaeger.Namespace), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned in the comment right before this review, I had two instances with the same name in different namespaces. One thing that was weird, is that only one console link was created. Turns out, console links are cluster-scoped, so, this logic here will make one instance remove another instance's console link. You can see this when running
Note how the namespace in the "deleting console link" log entries is empty, which was the clue that led me to check the CRD's scope: $ kubectl get crds consolelinks.console.openshift.io -o jsonpath='{.spec.scope}'
Cluster A couple of things to keep in mind then:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with 1), We should check for cluster admin before trying to create cluster object.
|
||
client.MatchingLabels(map[string]string{ | ||
"app.kubernetes.io/instance": jaeger.Name, | ||
"app.kubernetes.io/managed-by": "jaeger-operator", | ||
}), | ||
} | ||
list := &osconsolev1.ConsoleLinkList{} | ||
if err := r.rClient.List(ctx, list, opts...); err != nil { | ||
return tracing.HandleError(err, span) | ||
} | ||
|
||
inv := inventory.ForConsoleLinks(list.Items, desired) | ||
for _, d := range inv.Create { | ||
jaeger.Logger().WithFields(log.Fields{ | ||
"consoleLink": d.Name, | ||
"namespace": d.Namespace, | ||
}).Debug("creating console link") | ||
if err := r.client.Create(ctx, &d); err != nil { | ||
return tracing.HandleError(err, span) | ||
} | ||
} | ||
|
||
for _, d := range inv.Update { | ||
jaeger.Logger().WithFields(log.Fields{ | ||
"consoleLink": d.Name, | ||
"namespace": d.Namespace, | ||
}).Debug("updating console link") | ||
if err := r.client.Update(ctx, &d); err != nil { | ||
return tracing.HandleError(err, span) | ||
} | ||
} | ||
|
||
for _, d := range inv.Delete { | ||
jaeger.Logger().WithFields(log.Fields{ | ||
"consoleLink": d.Name, | ||
"namespace": d.Namespace, | ||
}).Debug("deleting console link") | ||
if err := r.client.Delete(ctx, &d); err != nil { | ||
return tracing.HandleError(err, span) | ||
} | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.