You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "hwzhuhao (via GitHub)" <gi...@apache.org> on 2023/11/13 12:13:52 UTC

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

hwzhuhao opened a new issue, #11539:
URL: https://github.com/apache/skywalking/issues/11539

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/skywalking/issues?q=is%3Aissue) 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? 
   
   ```go
   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
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org