[flyteagent] Default Service Config Using Round Robin Mechanism #6179

Future-Outlier · 2025-01-17T08:52:47Z

Tracking issue

Why are the changes needed?

This PR adds a default gRPC service configuration to the agent plugin that enables round-robin load balancing.
This ensures consistent and fair distribution of requests across multiple agent instances.

What changes were proposed in this pull request?

Added DefaultServiceConfig in the default agent configuration with round-robin load balancing policy
Default value set to: {"loadBalancingConfig": [{"round_robin":{}}]}
change agent endpoint from dns:///flyteagent.flyte.svc.cluster.local:8000 to k8s://flyteagent:8000

https://github.com/flyteorg/flyte/pull/6179/files#diff-98835a3d321cfb860916a188ca66ea98efe6c9f3a8314ea89ec2cfdf8ad50066R42-R47

How was this patch tested?

unit test and sandbox.

from flytekit import task, workflow
from flytekit.sensor.file_sensor import FileSensor

sensor = FileSensor(name="test_file_sensor")


@workflow()
def wf():
    sensor(path="s3://my-s3-bucket/a")

if __name__ == "__main__":
    import os
    from flytekit.clis.sdk_in_container import pyflyte
    from click.testing import CliRunner

    runner = CliRunner()
    path = os.path.realpath(__file__)
    
    for _ in range(100):  # Loop to trigger the workflow 100 times
        result = runner.invoke(pyflyte.main, ["run", "--remote", path, "wf"])
        print(result.output)

Labels

Please add one or more of the following labels to categorize your PR:

added: For new features.
changed: For changes in existing functionality.
deprecated: For soon-to-be-removed features.
removed: For features being removed.
fixed: For any bug fixed.
security: In case of vulnerabilities

This is important to improve the readability of release notes.

Setup process

agent-service:
defaultAgent:
  defaultTimeout: 10s
  endpoint: k8s://flyteagent:8000
  insecure: true
  timeouts:
    GetTask: 10s

Screenshots

Check all the applicable boxes

I updated the documentation accordingly.
All new and existing tests passed.
All commits are signed-off.

Related PRs

Docs link

Summary by Bito

Implementation of round-robin load balancing for Flyte agent service, including default configuration through DefaultServiceConfig field and transition from DNS-based to Kubernetes native service discovery. Updates include endpoint configuration modifications, security-related changes to shared secrets and checksums in sandbox and development manifests, with comprehensive test coverage to validate configuration values.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

Signed-off-by: Future-Outlier <[email protected]>

flyte-bot · 2025-01-17T08:53:10Z

Code Review Agent Run #141009

Actionable Suggestions - 1

flyteplugins/go/tasks/plugins/webapi/agent/config.go - 1
- Consider making service config configurable · Line 46-46

Review Details

Files reviewed - 2 · Commit Range: a433b33..a433b33
- flyteplugins/go/tasks/plugins/webapi/agent/config.go
- flyteplugins/go/tasks/plugins/webapi/agent/config_test.go
Files skipped - 0
Tools
- Golangci-lint (Linter) - ✖︎ Failed
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

AI Code Review powered by

flyte-bot · 2025-01-17T08:55:13Z

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Feature Improvement - Agent Service Load Balancing Enhancement	- `values.yaml` - Updated agent endpoint format to use k8s:// scheme - `values.yaml` - Updated agent endpoint format to use k8s:// scheme - `complete-agent.yaml` - Updated agent endpoint and security configurations - `complete.yaml` - Updated security configurations and checksums - `dev.yaml` - Updated security configurations and checksums - `config.go` - Added DefaultServiceConfig for round-robin load balancing - `config_test.go` - Added unit tests for default agent configuration

flyte-bot · 2025-01-17T08:55:15Z

flyteplugins/go/tasks/plugins/webapi/agent/config.go

+			Endpoint:             "",
+			Insecure:             true,
+			DefaultTimeout:       config.Duration{Duration: 10 * time.Second},
+			DefaultServiceConfig: `{"loadBalancingConfig": [{"round_robin":{}}]}`,


Consider making service config configurable

Consider making the default service config more configurable by moving it to a separate constant or configuration option rather than hardcoding the round robin load balancing config directly in the default config struct.

Code suggestion

Check the AI-generated fix before applying

const ( + DefaultServiceConfig = `{"loadBalancingConfig": [{"round_robin":{}}]}` ) Endpoint: "", Insecure: true, DefaultTimeout: config.Duration{Duration: 10 * time.Second}, - DefaultServiceConfig: `{"loadBalancingConfig": [{"round_robin":{}}]}`, + DefaultServiceConfig: DefaultServiceConfig,

Code Review Run #141009

Is this a valid issue, or was it incorrectly flagged by the Agent?

it was incorrectly flagged

codecov · 2025-01-17T08:58:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.06%. Comparing base (ab463c3) to head (0d2539f).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6179      +/-   ##
==========================================
- Coverage   37.06%   37.06%   -0.01%     
==========================================
  Files        1318     1318              
  Lines      132644   132644              
==========================================
- Hits        49164    49161       -3     
- Misses      79230    79234       +4     
+ Partials     4250     4249       -1

Flag	Coverage Δ
unittests-datacatalog	`51.58% <ø> (ø)`
unittests-flyteadmin	`54.31% <ø> (-0.03%)`	⬇️
unittests-flytecopilot	`30.99% <ø> (ø)`
unittests-flytectl	`62.29% <ø> (ø)`
unittests-flyteidl	`7.23% <ø> (ø)`
unittests-flyteplugins	`53.87% <ø> (+0.01%)`	⬆️
unittests-flytepropeller	`42.70% <ø> (ø)`
unittests-flytestdlib	`55.29% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Future-Outlier <[email protected]>

flyte-bot · 2025-01-18T04:35:49Z

Code Review Agent Run #f18921

Actionable Suggestions - 0

Review Details

Files reviewed - 5 · Commit Range: a433b33..0d2539f
- charts/flyte-binary/values.yaml
- charts/flyte-core/values.yaml
- docker/sandbox-bundled/manifests/complete-agent.yaml
- docker/sandbox-bundled/manifests/complete.yaml
- docker/sandbox-bundled/manifests/dev.yaml
Files skipped - 3
- charts/flyte-binary/README.md - Reason: Filter setting
- charts/flyte-core/README.md - Reason: Filter setting
- docs/user_guide/flyte_agents/developing_agents.md - Reason: Filter setting
Tools
- Golangci-lint (Linter) - ✖︎ Failed
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

AI Code Review powered by

Add FlyteAgent Default Service Config Using Round Robin Mechanism

a433b33

Signed-off-by: Future-Outlier <[email protected]>

flyte-bot reviewed Jan 17, 2025

View reviewed changes

update dns endpoint to k8s endpoint

0d2539f

Signed-off-by: Future-Outlier <[email protected]>

Future-Outlier requested a review from ppiegaze as a code owner January 18, 2025 03:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flyteagent] Default Service Config Using Round Robin Mechanism #6179

[flyteagent] Default Service Config Using Round Robin Mechanism #6179

Future-Outlier commented Jan 17, 2025 •

edited by flyte-bot

Loading

flyte-bot commented Jan 17, 2025 •

edited

Loading

Code Review Agent Run #141009

flyte-bot commented Jan 17, 2025 •

edited

Loading

Changelist by Bito

flyte-bot Jan 17, 2025

codecov bot commented Jan 17, 2025 •

edited

Loading

flyte-bot commented Jan 18, 2025 •

edited

Loading

Code Review Agent Run #f18921

[flyteagent] Default Service Config Using Round Robin Mechanism #6179

Are you sure you want to change the base?

[flyteagent] Default Service Config Using Round Robin Mechanism #6179

Conversation

Future-Outlier commented Jan 17, 2025 • edited by flyte-bot Loading

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

Labels

Setup process

Screenshots

Check all the applicable boxes

Related PRs

Docs link

Summary by Bito

flyte-bot commented Jan 17, 2025 • edited Loading

Code Review Agent Run #141009

flyte-bot commented Jan 17, 2025 • edited Loading

Changelist by Bito

flyte-bot Jan 17, 2025

Choose a reason for hiding this comment

codecov bot commented Jan 17, 2025 • edited Loading

Codecov Report

flyte-bot commented Jan 18, 2025 • edited Loading

Code Review Agent Run #f18921

Future-Outlier commented Jan 17, 2025 •

edited by flyte-bot

Loading

flyte-bot commented Jan 17, 2025 •

edited

Loading

flyte-bot commented Jan 17, 2025 •

edited

Loading

codecov bot commented Jan 17, 2025 •

edited

Loading

flyte-bot commented Jan 18, 2025 •

edited

Loading