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

EBS volume metrics collector daemon #3766

Merged
merged 7 commits into from Jun 30, 2023
Merged

EBS volume metrics collector daemon #3766

merged 7 commits into from Jun 30, 2023

Conversation

xxx0624
Copy link
Contributor

@xxx0624 xxx0624 commented Jun 26, 2023

Summary

  1. Add source code of the GRPC server hosted in the new daemon, EBS volume metrics collector, to the common library.
  2. Update the common lib vendor.
  3. Update agent vendor.

Implementation details

This daemon will host an unix socket based GRPC server which will implement one interface https://github.com/container-storage-interface/spec/blob/master/spec.md#nodegetvolumestats and can provide the capability for ECS Agent to get the volume stats for one given mounted EBS volume.

In this PR, it includes the GRPC server related logics only. And the implementation is same as the volume stats implementation of the existing AWS ebs csi driver implementation (https://github.com/kubernetes-sigs/aws-ebs-csi-driver/) except the following items -

  • the current implementation supports file based logging however the AWS ebs csi driver doesn't.
    • The klog is the log lib used by both current implementation and the AWS ebs csi driver. But AWS ebs csi driver forces all logs to go to the stdout/stderr.
  • the supported flags are different.
Usage of aws-ebs-csi-driver:
      --aws-sdk-debug-log                    To enable the aws sdk debug log level (default to false).
      --endpoint string                      Endpoint for the CSI driver server (default "unix://tmp/csi.sock")
      --extra-tags mapStringString           Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'
      --extra-volume-tags mapStringString    DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'
      --http-endpoint :8080                  The TCP network address where the HTTP server for metrics will listen (example: :8080). The default is empty string, which means the server is disabled.
      --k8s-tag-cluster-id string            ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).
      --log-flush-frequency duration         Maximum number of seconds between log flushes (default 5s)
      --log-json-info-buffer-size quantity   [Alpha] In JSON format with split output streams, the info messages can be buffered for a while to increase performance. The default value of zero bytes disables buffering. The size can be specified as number of bytes (512), multiples of 1000 (1K), multiples of 1024 (2Ki), or powers of those (3M, 4G, 5Mi, 6Gi). Enable the LoggingAlphaOptions feature gate to use this.
      --log-json-split-stream                [Alpha] In JSON format, write error messages to stderr and info messages to stdout. The default is to write a single stream to stdout. Enable the LoggingAlphaOptions feature gate to use this.
      --logging-format string                Sets the log format. Permitted formats: "json" (gated by LoggingBetaOptions), "text". (default "text")
      --logtostderr                          log to standard error instead of files. DEPRECATED: will be removed in a future release.
      --user-agent-extra string              Extra string appended to user agent.
  -v, --v Level                              number for the log level verbosity
      --version                              Print the version and exit.
      --vmodule pattern=N,...                comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)
      --volume-attach-limit int              Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes. If not specified, the value is approximated from the instance type. (default -1)
      --warn-on-invalid-tag                  To warn on invalid tags, instead of returning an error

how go.mod and go.sum are updated

  • Run go mod tidy and go mod vendor in both agent dir and ecs-agent dir already.
  • Verified that all new direct dependencies added to go.mod are same as the go.mod of agent.

how to start up the process (won't work until one following PR is out)

# This command below will force all logs (whose log level is not higher than 5) to the specified location
# Please refer this log conventions https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md#what-method-to-use

"/bin/csi-driver" "--endpoint=unix://tmp/csi.sock" "--log_file=/tmp/server.log" "-v=5" "--logtostderr=false" "--stderrthreshold=3"

Testing

New tests cover the changes: yes

Also

  • manually validated all tests which are included in the new directory are pass via make test under the new directory.

Description for the changelog

Add the EBS volume metrics collector.

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@xxx0624 xxx0624 requested a review from a team as a code owner June 26, 2023 22:09
@xxx0624 xxx0624 assigned fierlion and unassigned fierlion Jun 26, 2023
@xxx0624 xxx0624 requested a review from fierlion June 26, 2023 22:59
@xxx0624 xxx0624 force-pushed the dev branch 4 times, most recently from 164c953 to 6d984c3 Compare June 27, 2023 01:10
@xxx0624 xxx0624 force-pushed the dev branch 2 times, most recently from aa0a4b6 to 0ee049f Compare June 27, 2023 23:48
Copy link
Contributor

@mye956 mye956 left a comment

Choose a reason for hiding this comment

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

the supported flags are different

nit: Could we include what flags were added/removed/modified somewhere within the PR description?

Copy link
Contributor

@mye956 mye956 left a comment

Choose a reason for hiding this comment

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

the current implementation support file based logging via klog however the AWS ebs csi driver doesn't. The klog is the log lib used by the AWS ebs csi driver.

nit(non-blocking): this is a bit confusing to read at first, perhaps this could be reworded better to clarify whether or not klog is used by the AWS ebs csi driver?

@xxx0624
Copy link
Contributor Author

xxx0624 commented Jun 28, 2023

the supported flags are different

nit: Could we include what flags were added/removed/modified somewhere within the PR description?

Done.

the current implementation support file based logging via klog however the AWS ebs csi driver doesn't. The klog is the log lib used by the AWS ebs csi driver.

nit(non-blocking): this is a bit confusing to read at first, perhaps this could be reworded better to clarify whether or not klog is used by the AWS ebs csi driver?

Sure, updated.

ecs-agent/daemonimages/csidriver/Makefile Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/Makefile Outdated Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/Makefile Outdated Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/Makefile Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/options.go Outdated Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/options_test.go Outdated Show resolved Hide resolved
ecs-agent/daemonimages/csidriver/util/fs/fs.go Outdated Show resolved Hide resolved
amogh09
amogh09 previously approved these changes Jun 29, 2023
@xxx0624 xxx0624 merged commit 98722bd into aws:dev Jun 30, 2023
35 checks passed
@mye956 mye956 mentioned this pull request Jul 5, 2023
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.

None yet

6 participants