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 <zzxwill@gmail.com>

* address CI issues

Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>

* fix api issue

Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>

* fix ci

Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>

* fix ci

Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>

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
 	}

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 {

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,
 				},
 			}
 		}