From 820db96eae4e33bf28ff09952577c28d7511b796 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sun, 26 Dec 2021 20:38:19 +0800 Subject: [PATCH] Fix: change Grafana service type and remove domain parameter (#2996) * Fix: change Grafana service type and remove domain parameter Use Grafana service's external IP to visit the dashboard and remove the prameter domain Signed-off-by: Zheng Xi Zhou * address CI issues Signed-off-by: Zheng Xi Zhou * fix api issue Signed-off-by: Zheng Xi Zhou * fix ci Signed-off-by: Zheng Xi Zhou * fix ci Signed-off-by: Zheng Xi Zhou --- pkg/addon/addon.go | 36 +++++++++-------------------- pkg/addon/addon_test.go | 50 +++++------------------------------------ pkg/addon/helper.go | 31 +++++++++++++------------ 3 files changed, 32 insertions(+), 85 deletions(-) diff --git a/pkg/addon/addon.go b/pkg/addon/addon.go index 60a217b0b..1b0aabf6a 100644 --- a/pkg/addon/addon.go +++ b/pkg/addon/addon.go @@ -113,9 +113,10 @@ const ( // ObservabilityEnvironment contains the Observability addon's domain for each cluster type ObservabilityEnvironment struct { - Cluster string - Domain string - LoadBalancerIP string + Cluster string + Domain string + LoadBalancerIP string + ServiceExternalIP string } // ObservabilityEnvBindingValues is a list of ObservabilityEnvironment and will be used to render observability-env-binding.yaml @@ -135,14 +136,6 @@ const ( placement: clusterSelector: name: {{.Cluster}} - patch: - components: - - name: grafana - type: helm - traits: - - type: pure-ingress - properties: - domain: {{.Domain}} {{ end }} {{ end }}` @@ -512,12 +505,7 @@ func RenderApp(ctx context.Context, addon *InstallPackage, config *rest.Config, Type: "deploy2runtime", }) case addon.Name == ObservabilityAddon: - arg, ok := args[ObservabilityAddonDomainArg] - if !ok { - return nil, ErrorNoDomain - } - domain := arg.(string) - policies, err := preparePolicies4Observability(ctx, k8sClient, domain) + policies, err := preparePolicies4Observability(ctx, k8sClient) if err != nil { return nil, errors.Wrap(err, "fail to render the policies for Add-on Observability") } @@ -530,7 +518,7 @@ func RenderApp(ctx context.Context, addon *InstallPackage, config *rest.Config, }}, } - workflowSteps, err := prepareWorkflow4Observability(ctx, k8sClient, domain) + workflowSteps, err := prepareWorkflow4Observability(ctx, k8sClient) if err != nil { return nil, errors.Wrap(err, "fail to prepare the workflow for Add-on Observability") } @@ -618,7 +606,7 @@ func RenderDefinitionSchema(addon *InstallPackage) ([]*unstructured.Unstructured return schemaConfigmaps, nil } -func allocateDomainForAddon(ctx context.Context, k8sClient client.Client, domain string) ([]ObservabilityEnvironment, error) { +func allocateDomainForAddon(ctx context.Context, k8sClient client.Client) ([]ObservabilityEnvironment, error) { secrets, err := multicluster.ListExistingClusterSecrets(ctx, k8sClient) if err != nil { klog.Error(err, "failed to list existing cluster secrets") @@ -629,18 +617,16 @@ func allocateDomainForAddon(ctx context.Context, k8sClient client.Client, domain for i, secret := range secrets { cluster := secret.Name - domain := fmt.Sprintf("%s.%s", cluster, domain) envs[i] = ObservabilityEnvironment{ Cluster: cluster, - Domain: domain, } } return envs, nil } -func preparePolicies4Observability(ctx context.Context, k8sClient client.Client, domain string) ([]v1beta1.AppPolicy, error) { - clusters, err := allocateDomainForAddon(ctx, k8sClient, domain) +func preparePolicies4Observability(ctx context.Context, k8sClient client.Client) ([]v1beta1.AppPolicy, error) { + clusters, err := allocateDomainForAddon(ctx, k8sClient) if err != nil { return nil, err } @@ -670,8 +656,8 @@ func preparePolicies4Observability(ctx context.Context, k8sClient client.Client, return policies, nil } -func prepareWorkflow4Observability(ctx context.Context, k8sClient client.Client, domain string) ([]v1beta1.WorkflowStep, error) { - clusters, err := allocateDomainForAddon(ctx, k8sClient, domain) +func prepareWorkflow4Observability(ctx context.Context, k8sClient client.Client) ([]v1beta1.WorkflowStep, error) { + clusters, err := allocateDomainForAddon(ctx, k8sClient) if err != nil { return nil, err } diff --git a/pkg/addon/addon_test.go b/pkg/addon/addon_test.go index 7e5b49f81..617bd6959 100644 --- a/pkg/addon/addon_test.go +++ b/pkg/addon/addon_test.go @@ -32,7 +32,6 @@ import ( v1alpha12 "github.com/oam-dev/cluster-gateway/pkg/apis/cluster/v1alpha1" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -154,11 +153,9 @@ func TestRender(t *testing.T) { envs: []ObservabilityEnvironment{ { Cluster: "c1", - Domain: "a.com", }, { Cluster: "c2", - Domain: "b.com", }, }, tmpl: ObservabilityEnvBindingEnvTmpl, @@ -169,27 +166,11 @@ func TestRender(t *testing.T) { placement: clusterSelector: name: c1 - patch: - components: - - name: grafana - type: helm - traits: - - type: pure-ingress - properties: - domain: a.com - name: c2 placement: clusterSelector: name: c2 - patch: - components: - - name: grafana - type: helm - traits: - - type: pure-ingress - properties: - domain: b.com `, @@ -199,11 +180,9 @@ func TestRender(t *testing.T) { envs: []ObservabilityEnvironment{ { Cluster: "c1", - Domain: "a.com", }, { Cluster: "c2", - Domain: "b.com", }, }, tmpl: ObservabilityWorkflow4EnvBindingTmpl, @@ -345,17 +324,15 @@ func TestGetAddonStatus4Observability(t *testing.T) { Name: Convert2SecName(ObservabilityAddon), Namespace: types.DefaultKubeVelaNS, }, - Data: map[string][]byte{ - "domain": []byte("abc.com"), - }, + Data: map[string][]byte{}, } - addonIngress := &networkingv1.Ingress{ + addonService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: types.DefaultKubeVelaNS, Name: ObservabilityAddonEndpointComponent, }, - Status: networkingv1.IngressStatus{ + Status: corev1.ServiceStatus{ LoadBalancer: corev1.LoadBalancerStatus{ Ingress: []corev1.LoadBalancerIngress{ { @@ -387,8 +364,7 @@ func TestGetAddonStatus4Observability(t *testing.T) { assert.Equal(t, addonStatus.AddonPhase, enabling) // Addon is not installed in multiple clusters - assert.NoError(t, networkingv1.AddToScheme(scheme)) - k8sClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(addonApplication, addonSecret, addonIngress).Build() + k8sClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(addonApplication, addonSecret, addonService).Build() addonStatus, err = GetAddonStatus(context.Background(), k8sClient, ObservabilityAddon) assert.NoError(t, err) assert.Equal(t, addonStatus.AddonPhase, enabled) @@ -459,18 +435,6 @@ func TestRenderApp4Observability(t *testing.T) { }, }, args: map[string]interface{}{}, - application: "", - err: ErrorNoDomain, - }, - { - addon: InstallPackage{ - Meta: Meta{ - Name: "observability", - }, - }, - args: map[string]interface{}{ - "domain": "a.com", - }, application: `{"kind":"Application","apiVersion":"core.oam.dev/v1beta1","metadata":{"name":"addon-observability","namespace":"vela-system","creationTimestamp":null,"labels":{"addons.oam.dev/name":"observability"}},"spec":{"components":[],"policies":[{"name":"domain","type":"env-binding","properties":{"envs":null}}],"workflow":{"steps":[{"name":"deploy-control-plane","type":"apply-application-in-parallel"}]}},"status":{}}`, }, } @@ -517,10 +481,8 @@ func TestRenderApp4ObservabilityWithK8sData(t *testing.T) { Name: "observability", }, }, - args: map[string]interface{}{ - "domain": "a.com", - }, - application: `{"kind":"Application","apiVersion":"core.oam.dev/v1beta1","metadata":{"name":"addon-observability","namespace":"vela-system","creationTimestamp":null,"labels":{"addons.oam.dev/name":"observability"}},"spec":{"components":[],"policies":[{"name":"domain","type":"env-binding","properties":{"envs":[{"name":"test-secret","patch":{"components":[{"name":"grafana","traits":[{"properties":{"domain":"test-secret.a.com"},"type":"pure-ingress"}],"type":"helm"}]},"placement":{"clusterSelector":{"name":"test-secret"}}}]}}],"workflow":{"steps":[{"name":"deploy-control-plane","type":"apply-application-in-parallel"},{"name":"test-secret","type":"deploy2env","properties":{"env":"test-secret","parallel":true,"policy":"domain"}}]}},"status":{}}`, + args: map[string]interface{}{}, + application: `{"kind":"Application","apiVersion":"core.oam.dev/v1beta1","metadata":{"name":"addon-observability","namespace":"vela-system","creationTimestamp":null,"labels":{"addons.oam.dev/name":"observability"}},"spec":{"components":[],"policies":[{"name":"domain","type":"env-binding","properties":{"envs":[{"name":"test-secret","placement":{"clusterSelector":{"name":"test-secret"}}}]}}],"workflow":{"steps":[{"name":"deploy-control-plane","type":"apply-application-in-parallel"},{"name":"test-secret","type":"deploy2env","properties":{"env":"test-secret","parallel":true,"policy":"domain"}}]}},"status":{}}`, }, } for _, tc := range testcases { diff --git a/pkg/addon/helper.go b/pkg/addon/helper.go index 48b40cd68..16b32b607 100644 --- a/pkg/addon/helper.go +++ b/pkg/addon/helper.go @@ -24,7 +24,6 @@ import ( "k8s.io/klog/v2" v1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/rest" @@ -117,9 +116,10 @@ func GetAddonStatus(ctx context.Context, cli client.Client, name string) (Status access = fmt.Sprintf("Visiting URL: %s, IP: %s", o.Domain, o.LoadBalancerIP) } clusters[o.Cluster] = map[string]interface{}{ - "domain": o.Domain, - "loadBalancerIP": o.LoadBalancerIP, - "access": access, + "domain": o.Domain, + "loadBalancerIP": o.LoadBalancerIP, + "access": access, + "serviceExternalIP": o.ServiceExternalIP, } } return Status{AddonPhase: enabled, AppStatus: &app.Status, Clusters: clusters}, nil @@ -134,14 +134,14 @@ func GetAddonStatus(ctx context.Context, cli client.Client, name string) (Status // GetObservabilityAccessibilityInfo will get the accessibility info of addon in local cluster and multiple clusters func GetObservabilityAccessibilityInfo(ctx context.Context, k8sClient client.Client, domain string) ([]ObservabilityEnvironment, error) { - domains, err := allocateDomainForAddon(ctx, k8sClient, domain) + domains, err := allocateDomainForAddon(ctx, k8sClient) if err != nil { return nil, err } obj := new(unstructured.Unstructured) - obj.SetKind("Ingress") - obj.SetAPIVersion("networking.k8s.io/v1") + obj.SetKind("Service") + obj.SetAPIVersion("v1") key := client.ObjectKeyFromObject(obj) key.Namespace = types.DefaultKubeVelaNS key.Name = ObservabilityAddonEndpointComponent @@ -153,29 +153,28 @@ func GetObservabilityAccessibilityInfo(ctx context.Context, k8sClient client.Cli if err := k8sClient.Get(readCtx, key, obj); err != nil { return nil, err } - var ingress networkingv1.Ingress + var svc v1.Service data, err := obj.MarshalJSON() if err != nil { return nil, err } - if err := json.Unmarshal(data, &ingress); err != nil { + if err := json.Unmarshal(data, &svc); err != nil { return nil, err } - if ingress.Status.LoadBalancer.Ingress != nil && len(ingress.Status.LoadBalancer.Ingress) == 1 { - domains[i].LoadBalancerIP = ingress.Status.LoadBalancer.Ingress[0].IP + if svc.Status.LoadBalancer.Ingress != nil && len(svc.Status.LoadBalancer.Ingress) == 1 { + domains[i].ServiceExternalIP = svc.Status.LoadBalancer.Ingress[0].IP } } // set domain for the cluster if there is no child clusters if len(domains) == 0 { - var ingress networkingv1.Ingress - if err := k8sClient.Get(ctx, client.ObjectKey{Name: ObservabilityAddonEndpointComponent, Namespace: types.DefaultKubeVelaNS}, &ingress); err != nil { + var svc v1.Service + if err := k8sClient.Get(ctx, client.ObjectKey{Name: ObservabilityAddonEndpointComponent, Namespace: types.DefaultKubeVelaNS}, &svc); err != nil { return nil, err } - if ingress.Status.LoadBalancer.Ingress != nil && len(ingress.Status.LoadBalancer.Ingress) == 1 { + if svc.Status.LoadBalancer.Ingress != nil && len(svc.Status.LoadBalancer.Ingress) == 1 { domains = []ObservabilityEnvironment{ { - Domain: domain, - LoadBalancerIP: ingress.Status.LoadBalancer.Ingress[0].IP, + ServiceExternalIP: svc.Status.LoadBalancer.Ingress[0].IP, }, } }