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

✨ Expose QPS and Burst flags on agent command line #56

Merged
merged 1 commit into from
May 17, 2024

Conversation

MikeSpreitzer
Copy link
Contributor

@MikeSpreitzer MikeSpreitzer commented May 17, 2024

Summary

This PR adds command line flags to the OCM Status Add-On Agent for client-side self-restraint, in terms of QPS and Burst, for accessing the local cluster (WEC) and for accessing the hub (ITS).

Related issue(s)

@MikeSpreitzer
Copy link
Contributor Author

After this change:

mspreitz@mjs13 ocm-status-addon % go run ./cmd/ocm-status-addon agent -h
Start the addon agent

Usage:
  addon agent [flags]

Flags:
      --addon-name string                 name of the addon. (default "status")
      --addon-namespace string            Installation namespace of addon.
      --cluster-name string               Name of spoke cluster.
      --component-namespace string        Namespace of the component.
      --enable-leader-election            Enables the leader election for the controller
  -h, --help                              help for agent
      --hub-burst int                     Allowed burst in requests/sec for accessing the hub (default 10)
      --hub-kubeconfig string             Location of kubeconfig file to connect to hub cluster.
      --hub-qps float                     Max average requests/sec for accessing the hub (default 5)
      --kubeconfig string                 Location of the master configuration file to run from.
      --local-burst int                   Allowed burst in requests/sec for accessing the local cluster (default 10)
      --local-qps float                   Max average requests/sec for accessing the local cluster (default 5)
      --zap-devel                         Development Mode defaults(encoder=consoleEncoder,logLevel=Debug,stackTraceLevel=Warn). Production Mode defaults(encoder=jsonEncoder,logLevel=Info,stackTraceLevel=Error) (default true)
      --zap-encoder encoder               Zap log encoding (one of 'json' or 'console')
      --zap-log-level level               Zap Level to configure the verbosity of logging. Can be one of 'debug', 'info', 'error', or any integer value > 0 which corresponds to custom debug levels of increasing verbosity
      --zap-stacktrace-level level        Zap Level at and above which stacktraces are captured (one of 'info', 'error', 'panic').
      --zap-time-encoding time-encoding   Zap time encoding (one of 'epoch', 'millis', 'nano', 'iso8601', 'rfc3339' or 'rfc3339nano'). Defaults to 'epoch'.

Global Flags:
      --log-flush-frequency duration   Maximum number of seconds between log flushes (default 5s)
      --logging-format string          Sets the log format. Permitted formats: "text". (default "text")
  -v, --v Level                        number for the log level verbosity
      --vmodule pattern=N,...          comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)

@@ -0,0 +1,88 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of a file in kubestellar/kubestellar#2165 . Once we decide how to remove the circular dependency between ks/ks and ks/OSA, we can remove one copy in favor of importing.

@pdettori
Copy link
Collaborator

@MikeSpreitzer: please note that if we want to expose agent flags the status add-on chart, these values need to be provided to the add-on controller and from there set in the ManifestConfig - see

func GetDefaultValues(cluster *clusterv1.ManagedCluster,
addon *addonapiv1alpha1.ManagedClusterAddOn) (addonfactory.Values, error) {
installNamespace := addon.Spec.InstallNamespace
if len(installNamespace) == 0 {
installNamespace = InstallationNamespace
}
image := os.Getenv("STATUS_ADDDON_IMAGE_NAME")
if len(image) == 0 {
image = DefaultStatusAddOnImage
}
manifestConfig := struct {
KubeConfigSecret string
ClusterName string
AddonInstallNamespace string
Image string
}{
KubeConfigSecret: fmt.Sprintf("%s-hub-kubeconfig", addon.Name),
AddonInstallNamespace: installNamespace,
ClusterName: cluster.Name,
Image: image,
}
return addonfactory.StructToValues(manifestConfig), nil
}
. The controller builds the manifestwork for the add-on by applying those values to the templates defined in https://github.com/kubestellar/ocm-status-addon/tree/main/pkg/controller/manifests/templates.

Therefore, arguments to propagate to the agent should be templated in

args:
- "agent"
- "--hub-kubeconfig=/var/run/hub/kubeconfig"
- "--cluster-name={{ .ClusterName }}"
- "--addon-namespace={{ .AddonInstallNamespace }}"
.

@MikeSpreitzer
Copy link
Contributor Author

Yes, there is more to do. Can we do it in a follow-on PR?

@pdettori
Copy link
Collaborator

ok with me

Copy link
Collaborator

@pdettori pdettori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@pdettori pdettori merged commit a9e4606 into kubestellar:main May 17, 2024
7 checks passed
@MikeSpreitzer MikeSpreitzer deleted the expose-qps branch May 17, 2024 21:34
@MikeSpreitzer
Copy link
Contributor Author

@pdettori: thanks for the pointers about the templating and variables. I am mystified about some. I see https://github.com/kubestellar/ocm-status-addon/blob/v0.2.0-rc8/cmd/ocm-status-addon/main.go#L100-L107 , and it looks like https://github.com/kubestellar/ocm-status-addon/blob/v0.2.0-rc8/cmd/ocm-status-addon/main.go#L102-L106 is moot because there is no AddOnDeploymentConfig , and do not see the source of many variables I see referenced in https://github.com/kubestellar/ocm-status-addon/blob/v0.2.0-rc8/pkg/controller/manifests/templates/deployment.yaml, such as HTTPSProxy and Tolerations.

@MikeSpreitzer
Copy link
Contributor Author

MikeSpreitzer commented May 18, 2024

The mysterious references may be outdated junk; see open-cluster-management-io/addon-framework#270 .

The template might work because of subtleties of Go template functionality. https://pkg.go.dev/text/[email protected]#Template.Option is relevant here, and the OCM add-on framework documentation does not mention which option is used. Looking in the OCM code, I see that Template.Option is not invoked; this means that the default behavior applies. The doc for Template.Options says that the default behavior when a template references a key that is not in the map is to "Do nothing and continue execution". It is not obvious to me what this means, but it might mean that an enclosing "if" pipeline expands to the empty string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants