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

extend log command to set severity for components running in managed clusters #41

Open
gianlucam76 opened this issue Jan 18, 2023 · 6 comments
Labels
help wanted Extra attention is needed

Comments

@gianlucam76
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently, using sveltosctl, it is possible to change log severity for components running in the management cluster.
There is though no support to change log severity for components running in managed clusters.

Describe the solution you'd like
Extend sveltosctl log-settings command to set log severity for components running in managed cluster. Cluster namespace/name must be requested to user.

Describe alternatives you've considered
Alternative today is to use kubectl and post DebuggingConfiguration default instance in managed clusters.

@gianlucam76 gianlucam76 added the help wanted Extra attention is needed label Jan 18, 2023
@AdamSadek
Copy link
Contributor

AdamSadek commented Sep 2, 2023

Hi @gianlucam76,

This is somewhat of a grey area for me, however, I was wondering if it's possible to create a Custom Resource Definition (CRD) that allows users to specify the log severity for components in managed clusters? I think you would also need to create some controller & logic controller(s) with it to help handle what gets updated in that CRD.

@gianlucam76
Copy link
Member Author

gianlucam76 commented Sep 2, 2023

Hi @AdamSadek thanks for your comment.

what you are saying is correct and sveltos has part of it.

For instance there is this CRD where customer can specify:

  1. Component
  2. log severity

And then each Sveltos components uses this method to watch for DebuggingConfiguration instance changes and dynamically adjust the log verbosity accordingly.
Logic to watch and react to change is implemented in the method. So all components need to simply invoked it. For instance, sveltos-agent does it here.

Today, sveltosctl can set DebuggingConfiguration default instance in the management cluster and the Sveltos components running in the management cluster adjust their log verbosity.

Sveltos though has also components running in the managed cluster (sveltos-agent and drift-detection-manager). But sveltosctl has no capability to update DebuggingConfiguration in the managed clusters.

The goal of this enhancement is to modify sveltosctl so to configure DebuggingConfiguration also in managed cluster.
That means for instance changing set to accept cluster namespace and cluster name as optional args. If specified, instead of setting DebuggingConfiguration default instance in the management cluster, set it in the managed cluster specified.

Let me know what you think. Thank you again

@AdamSadek
Copy link
Contributor

AdamSadek commented Sep 2, 2023

Thank you for providing more context @gianlucam76. This is the approach I'm currently seeing:

  1. Modify the log-level set command to include optional arguments as you mentioned for cluster namespace and cluster name, i.e:
  • sveltosctl log-level set --component=<name> (--info|--debug|--verbose) [--namespace=<namespace>] [--clusterName=<cluster-name>]
  • Also updating the Options
  1. We also need to parse those new args:
namespace := parsedArgs["--namespace"].(string)
clusterName:= parsedArgs["--clusterName"].(string)
  1. At the end where you return the updateDebuggingConfiguration, we will add a conditional statement that will check whether we should configure DebuggingConfiguration in the management cluster or the specified managed cluster.
    i.e:
if namespace != "" && clusterName != "" {
    return updateDebuggingConfigurationInManaged(ctx, logSeverity, component, namespace, clusterName) // Need to create this func - see (4.)
}
return updateDebuggingConfiguration(ctx, logSeverity, component)
  1. If it's a managed cluster I guess you would need some type of new implementation to be able to update the DebuggingConfiguration resource in those namespace of the managed cluster? - I guess this is the hard part :D
  • I'm guessing for this part that you would need to interact with the Kubernetes/svelto API of the managed cluster using the provided parameters that we added (namespace and clusterName)
  • Most likely you'd need to connect to the server of managed cluster itself (might need some credential set up as well, possibly with kubeconfig🤔)
  • Create a k8's client to communicate with the managed cluster's API server? (not sure if Go has the appropriate libs to do that, I think most likely yes) - it looks like you might've already set all that up?
  • Finally updating the DebuggingConfiguration resource in the specified namespace of the managed cluster, with the client that has been created. Like here
  • i.e
dc := &libsveltosv1alpha1.DebuggingConfiguration{
        ObjectMeta: metav1.ObjectMeta{
            ClusterName:   clusterName,
            Namespace: namespace,
        },
    }

This is a very brief idea of what I had in mind - not really sure if it will work, and I hope I haven't missed anything in my examples.
It might also be a bit all over the place, but hopefully useful in some way. Let me know what you think! :)

@gianlucam76
Copy link
Member Author

Thank you @AdamSadek, what you suggested looks great.

Few comments.

Since sveltos manages two different type of managed clusters:

  • clusterAPI powered cluster
  • clusters registered with sveltos

we need to ask also clusterType (defined here) on top of cluster namespace/name.

Regarding point 5, libsveltos has this method that allows to get client to talk to API Server in the managed cluster. Where:

  • adminNamespace, adminName are empty in this case, as platform admin is doing this operation (not a tenant admin);
  • c client.Client is the management cluster client (utils.GetAccessInstance().GetClient() returns the client to access management cluster).

Maybe we could change collectLogLevelConfiguration to accept the DebuggingConfiguration (and do same for updateLogLevelConfiguration). Or something similar.

So we can fetch DebuggingConfiguration from either management cluster or managed cluster with the logic you suggested.

Today, collectLogLevelConfiguration method assumes it needs to talk to the management cluster only. So with following code it gets DebuggingConfiguration from management cluster.

	instance := utils.GetAccessInstance()

	dc, err := instance.GetDebuggingConfiguration(ctx)
	if err != nil {
		if apierrors.IsNotFound(err) {
			return make([]*componentConfiguration, 0), nil
		}
		return nil, err
	}

PS: Really good job finding all those info on your own. I could have added more info on the issue (apologies).

@AdamSadek
Copy link
Contributor

AdamSadek commented Sep 2, 2023

Thank you and no worries! @gianlucam76

So in-addition to the cluster namespace and name, we would need to also add the clusterType👍

It's also good to see that there's already a method that deals with the API server communication. 👍

What you suggested sounds great. Making tweaks to the collectLogLevelConfiguration & updateLogLevelConfiguration, to accept the DebuggingConfiguration and then deal with whether it's a management or managed cluster with some logic.

Would you have a chronological procedure in mind on making these changes? - some sort of Acceptance Criteria (AC)?

Also for testing, do you test the changes right after merging into dev ?

@gianlucam76
Copy link
Member Author

Hi @AdamSadek regarding testing, unit tests can be added as part of the PR. Set command already has some tests. You can add others based on your change.

Then sveltosctl has "make lint" and "make test". After you make changes you can run those locally to make sure all is good (those are also part of each PR's CI).

If you want to test on a test setup, you can clone this repo. Then run "make quickstart" which will creates:

  1. management cluster with clusterAPI and projectsveltos
  2. a managed cluster ("kubectl get cluster" -A to see it)

at this point you can test your sveltosctl changes by setting "sveltos-agent" component log severity in the managed cluster and verify it is set as expected.

repo in test/fv/workload_kubeconfig you can find kubeconfig to access the managed cluster so you can run "KUBECONFIG=<addon-controller_location>/ test/fv/workload_kubeconfig kubectl get debuggingconfiguration -o yaml" to verify it is set.

Thank you and please do not hesitate if you have any other question. Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants