skywalking icon indicating copy to clipboard operation
skywalking copied to clipboard

[Feature] [Operator] Optimize the InjectStorage method of OAPServerReconciler to improve the efficiency of Reconcile execution

Open hwzhuhao opened this issue 2 years ago • 0 comments

Search before asking

  • [X] I had searched in the issues and found no similar feature requirement.

Description

the skywalking-swck version is v0.8.0. when i create an oapserver instance, the OAPServerReconciler will excute reconcile method, I found the InjectStorage method is only used to check storage name and get storage info like SW_STORAGE_ES_HTTP_PROTOCOL,SW_STORAGE, SW_ES_USER, SW_ES_PASSWORD and so on to append to oapserver.spec.config field. But this method and subsequent methods does not perform update the oapserver instance to kube-apiserver. So these updates are invalid, can we optimize the InjectStorage method to only check whether the storage name is exists?

func (r *OAPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
	log := runtimelog.FromContext(ctx)
	log.Info("=====================oapserver reconcile started================================")

	oapServer := operatorv1alpha1.OAPServer{}
	if err := r.Client.Get(ctx, req.NamespacedName, &oapServer); err != nil {
		return ctrl.Result{}, client.IgnoreNotFound(err)
	}
	ff, err := r.FileRepo.GetFilesRecursive("templates")
	if err != nil {
		log.Error(err, "failed to load resource templates")
		return ctrl.Result{}, err
	}
	app := kubernetes.Application{
		Client:   r.Client,
		FileRepo: r.FileRepo,
		CR:       &oapServer,
		GVK:      operatorv1alpha1.GroupVersion.WithKind("OAPServer"),
		Recorder: r.Recorder,
	}

	r.InjectStorage(ctx, log, &oapServer)

	if err := app.ApplyAll(ctx, ff, log); err != nil {
		return ctrl.Result{}, err
	}

	if err := r.checkState(ctx, log, &oapServer); err != nil {
		l.Error(err, "failed to check sub resources state")
		return ctrl.Result{}, err
	}

	return ctrl.Result{RequeueAfter: schedDuration}, nil
}

// InjectStorage Inject Storage
func (r *OAPServerReconciler) InjectStorage(ctx context.Context, log logr.Logger, oapServer *operatorv1alpha1.OAPServer) {
	if oapServer.Spec.StorageConfig.Name == "" {
		return
	}
	storage := &operatorv1alpha1.Storage{}
	err := r.Client.Get(ctx, client.ObjectKey{Namespace: oapServer.Namespace, Name: oapServer.Spec.StorageConfig.Name}, storage)
	if err == nil {
		r.ConfigStorage(ctx, log, storage, oapServer)
		log.Info("success inject storage")
	} else {
		log.Info("fail inject storage")
	}
}

func (r *OAPServerReconciler) ConfigStorage(ctx context.Context, log logr.Logger, s *operatorv1alpha1.Storage, o *operatorv1alpha1.OAPServer) {
	user, tls := s.Spec.Security.User, s.Spec.Security.TLS
	SwStorageEsHTTPProtocol := "http"
	SwEsUser := ""
	SwEsPassword := ""
	SwStorageEsSslJksPath := ""
	SwStorageEsSslJksPass := "skywalking"
	SwStorageEsClusterNodes := ""
	o.Spec.StorageConfig.Storage = *s
	if user.SecretName != "" {
		if user.SecretName == "default" {
			SwEsUser = "elastic"
			SwEsPassword = "changeme"
		} else {
			usersecret := &core.Secret{}
			if err := r.Client.Get(ctx, client.ObjectKey{Namespace: s.Namespace, Name: user.SecretName}, usersecret); err != nil && !apierrors.IsNotFound(err) {
				log.Info("fail get usersecret ")
			}
			for k, v := range usersecret.Data {
				if k == "username" {
					SwEsUser = string(v)
				} else if k == "password" {
					SwEsPassword = string(v)
				}
			}
		}
	}
	if tls {
		SwStorageEsHTTPProtocol = "https"
		SwStorageEsSslJksPath = "/skywalking/p12/storage.p12"
		SwStorageEsClusterNodes = "skywalking-storage"
	} else {
		SwStorageEsClusterNodes = s.Name + "-" + s.Spec.Type
	}

	o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE", Value: s.Spec.Type})
	if user.SecretName != "" {
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_ES_USER", Value: SwEsUser})
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_ES_PASSWORD", Value: SwEsPassword})
	}
	if tls {
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_SSL_JKS_PATH", Value: SwStorageEsSslJksPath})
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_SSL_JKS_PASS", Value: SwStorageEsSslJksPass})
	}
	if apiequal.Semantic.DeepDerivative(s.Spec.ConnectType, "external") {
		parseurl, _ := url.Parse(s.Spec.ConnectAddress)
		SwStorageEsHTTPProtocol = parseurl.Scheme
		SwStorageEsClusterNodes = parseurl.Host
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_HTTP_PROTOCOL", Value: SwStorageEsHTTPProtocol})
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_CLUSTER_NODES", Value: SwStorageEsClusterNodes})
	} else {
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_HTTP_PROTOCOL", Value: SwStorageEsHTTPProtocol})
		o.Spec.Config = append(o.Spec.Config, core.EnvVar{Name: "SW_STORAGE_ES_CLUSTER_NODES", Value: SwStorageEsClusterNodes + ":9200"})
	}
}

Use case

No response

Related issues

No response

Are you willing to submit a pull request to implement this on your own?

  • [X] Yes I am willing to submit a pull request on my own!

Code of Conduct

hwzhuhao avatar Nov 13 '23 12:11 hwzhuhao