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

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

Open
3 tasks done
hwzhuhao opened this issue Nov 13, 2023 · 0 comments
Assignees
Labels
feature New feature SWCK Apache SkyWalking Cloud on Kubernetes

Comments

@hwzhuhao
Copy link

Search before asking

  • 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?

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

Code of Conduct

@hwzhuhao hwzhuhao added the feature New feature label Nov 13, 2023
@wu-sheng wu-sheng added the SWCK Apache SkyWalking Cloud on Kubernetes label Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature SWCK Apache SkyWalking Cloud on Kubernetes
Projects
None yet
Development

No branches or pull requests

4 participants