refactor maestro agent flags. #112

morvencao · 2024-06-06T04:07:32Z

Refactor maestro agent flags:

Enable manifest & manifestbundle codec for maestro agent.
Disable leader election by default, but can be enabled explicitly by flags when running agent in multiple instances.

morvencao · 2024-06-06T04:08:26Z

cmd/maestro/agent/cmd.go

@@ -70,7 +70,6 @@ func runAgent(cmd *cobra.Command, args []string) {
 	cfg := spoke.NewWorkAgentConfig(commonOptions, agentOption)
 	cmdConfig := commonOptions.CommoOpts.
 		NewControllerCommandConfig("maestro-agent", version.Get(), cfg.RunWorkloadAgent)
-	cmdConfig.DisableLeaderElection = true


@clyang82 I found this was added in #20, any reason to disable leader election explicitly?

would you consider to use cmdConfig.NewCommandWithContext(context.TODO()) to build command, e.g. https://github.com/open-cluster-management-io/ocm/blob/main/pkg/cmd/spoke/work.go#L24

so we can get more basic flags from library go

no specific reason to disable it at the beginning. just make it works for initial version. We need to enable it based on the current case.

Updated this to following https://github.com/open-cluster-management-io/ocm/blob/main/pkg/cmd/spoke/work.go#L24

One question: we have added some flag alias for maestro agent, which are different from work-agent. Eg. --consumer-name instead of --spoke-cluster-name? do we still need these alias flags?

what does election leader provide that advisory locks from postgres do not?

locks give us concurrency and parallelism in execution. leader election ties us to a single pod at a time and has proven troublesome with CS in the past.

ah, i think this is in the context of the agent, which would be in a cluster without the postgres database. nevermind my question.

yes, the agent is using in-cluster leader election.

markturansky · 2024-06-06T12:51:51Z

i'd advise against election leader and instead use the advisory locks provided by postgres.

morvencao · 2024-06-07T03:52:20Z

Did some testing with new agent:

No permission(RBAC) issue.
DisableLeaderElection flags works.
MaxJSONRawLength works.

/cc @clyang82 @skeeey

Signed-off-by: morvencao <[email protected]>

clyang82 · 2024-06-07T06:28:03Z

how about standalone maestro agent, does it work?

clyang82 · 2024-06-07T06:30:31Z

cmd/maestro/agent/cmd.go

+	agentOption.CloudEventsClientCodecs = []string{"manifest", "manifestbundle"}
+	cfg := spoke.NewWorkAgentConfig(commonOptions, agentOption)
+	cmdConfig := commonOptions.CommoOpts.
+		NewControllerCommandConfig("work-agent", version.Get(), cfg.RunWorkloadAgent)


maybe use maestro-agent here.

updated to maestro-agent.

clyang82 · 2024-06-07T06:31:20Z

cmd/maestro/agent/cmd.go

-		[]string{"manifest"}, "The codecs of the agent client. The valid codecs are manifest and manifestbundle")
-
+	fs.BoolVar(&commonOptions.CommoOpts.CmdConfig.DisableLeaderElection, "disable-leader-election",
+		true, "Disable leader election.")


How long will take to start the agent if enable leader election by default?

I didn't see difference from the case when leader election is disabled.

so can we enable the leader election by default?

as we discussed, we disable leader election by default.

morvencao · 2024-06-07T08:09:57Z

@clyang82
maestro-agent can run as standalone:

# go run ./cmd/maestro/main.go agent --kubeconfig=/tmp/kubeconfig  --workload-source-config=/tmp/config.yaml --workload-source-driver=mqtt --cloudevents-client-id=cluster1-agent --consumer-name=cluster1
W0607 08:08:33.125558  385318 cmd.go:245] Using insecure, self-signed certificates
I0607 08:08:33.544479  385318 observer_polling.go:159] Starting file observer
W0607 08:08:33.545032  385318 builder.go:260] unable to identify the current namespace for events: open /var/run/secrets/kubernetes.io/serviceaccount/namespace: no such file or directory
W0607 08:08:33.554245  385318 builder.go:267] unable to get owner reference (falling back to namespace): pods "test" not found
I0607 08:08:33.554465  385318 builder.go:299] maestro-agent version v0.0.0-master+$Format:%H$-$Format:%H$
I0607 08:08:34.038681  385318 secure_serving.go:57] Forcing use of http/1.1 only
W0607 08:08:34.038755  385318 secure_serving.go:69] Use of insecure cipher 'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256' detected.
W0607 08:08:34.038763  385318 secure_serving.go:69] Use of insecure cipher 'TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256' detected.
I0607 08:08:34.042255  385318 factory.go:58] Executor caches enabled: false
{"level":"info","ts":1717747714.0423555,"logger":"fallback","caller":"[email protected]/protocol.go:124","msg":"subscribing to topics: map[sources/maestro/consumers/cluster/sourceevents:{1 0 false false}]"}
I0607 08:08:34.043134  385318 requestheader_controller.go:169] Starting RequestHeaderAuthRequestController
I0607 08:08:34.043191  385318 shared_informer.go:311] Waiting for caches to sync for RequestHeaderAuthRequestController
I0607 08:08:34.043207  385318 configmap_cafile_content.go:202] "Starting controller" name="client-ca::kube-system::extension-apiserver-authentication::client-ca-file"
I0607 08:08:34.043255  385318 configmap_cafile_content.go:202] "Starting controller" name="client-ca::kube-system::extension-apiserver-authentication::requestheader-client-ca-file"
I0607 08:08:34.043282  385318 shared_informer.go:311] Waiting for caches to sync for client-ca::kube-system::extension-apiserver-authentication::client-ca-file
I0607 08:08:34.043309  385318 shared_informer.go:311] Waiting for caches to sync for client-ca::kube-system::extension-apiserver-authentication::requestheader-client-ca-file
I0607 08:08:34.043818  385318 dynamic_serving_content.go:132] "Starting controller" name="serving-cert::/tmp/serving-cert-491928347/tls.crt::/tmp/serving-cert-491928347/tls.key"
W0607 08:08:34.044270  385318 agentoptions.go:63] the agent broadcast topic not set, fall back to the agent events topic
I0607 08:08:34.044494  385318 base_controller.go:67] Waiting for caches to sync for AvailableStatusController
I0607 08:08:34.044524  385318 base_controller.go:67] Waiting for caches to sync for AppliedManifestWorkFinalizer
I0607 08:08:34.044554  385318 base_controller.go:67] Waiting for caches to sync for ManifestWorkAddFinalizerController
I0607 08:08:34.044617  385318 base_controller.go:67] Waiting for caches to sync for AppliedManifestWorkController
I0607 08:08:34.044639  385318 base_controller.go:67] Waiting for caches to sync for ManifestWorkAgent
I0607 08:08:34.044655  385318 base_controller.go:67] Waiting for caches to sync for UnManagedAppliedManifestWork
I0607 08:08:34.044671  385318 base_controller.go:67] Waiting for caches to sync for ManifestWorkFinalizer
I0607 08:08:34.044811  385318 secure_serving.go:213] Serving securely on [::]:8443
I0607 08:08:34.044865  385318 tlsconfig.go:240] "Starting DynamicServingCertificateController"
W0607 08:08:34.045282  385318 agentoptions.go:63] the agent broadcast topic not set, fall back to the agent events topic
...

Signed-off-by: morvencao <[email protected]>

clyang82 · 2024-06-11T02:38:01Z

/ok-to-test

clyang82

LGTM

morvencao commented Jun 6, 2024

View reviewed changes

morvencao force-pushed the br_enhance_agent branch 2 times, most recently from 5ef1b8f to 5507940 Compare June 6, 2024 10:40

morvencao force-pushed the br_enhance_agent branch from 5507940 to 7cfd7d1 Compare June 7, 2024 02:50

morvencao changed the title ~~enable leader election and manifest & manifestbundle codec for agent.~~ refactor maestro agent flags. Jun 7, 2024

morvencao force-pushed the br_enhance_agent branch 2 times, most recently from 5844132 to 8a14c54 Compare June 7, 2024 03:32

refactor maestro agent flags.

ad90aaa

Signed-off-by: morvencao <[email protected]>

morvencao force-pushed the br_enhance_agent branch from 8a14c54 to ad90aaa Compare June 7, 2024 04:02

clyang82 reviewed Jun 7, 2024

View reviewed changes

change component name.

b04e66b

Signed-off-by: morvencao <[email protected]>

Merge branch 'main' into br_enhance_agent

fc8485d

clyang82 approved these changes Jun 11, 2024

View reviewed changes

clyang82 merged commit ae149df into openshift-online:main Jun 11, 2024
5 checks passed

morvencao deleted the br_enhance_agent branch June 11, 2024 06:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor maestro agent flags. #112

refactor maestro agent flags. #112

morvencao commented Jun 6, 2024 •

edited

Loading

morvencao Jun 6, 2024

skeeey Jun 6, 2024 •

edited

Loading

clyang82 Jun 6, 2024

morvencao Jun 6, 2024 •

edited

Loading

markturansky Jun 6, 2024

markturansky Jun 6, 2024

morvencao Jun 7, 2024

markturansky commented Jun 6, 2024

morvencao commented Jun 7, 2024 •

edited

Loading

clyang82 commented Jun 7, 2024

clyang82 Jun 7, 2024

morvencao Jun 7, 2024

clyang82 Jun 7, 2024

morvencao Jun 7, 2024

clyang82 Jun 7, 2024

morvencao Jun 11, 2024

morvencao commented Jun 7, 2024

clyang82 commented Jun 11, 2024

clyang82 left a comment

refactor maestro agent flags. #112

refactor maestro agent flags. #112

Conversation

morvencao commented Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

skeeey Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morvencao Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markturansky commented Jun 6, 2024

morvencao commented Jun 7, 2024 • edited Loading

clyang82 commented Jun 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morvencao commented Jun 7, 2024

clyang82 commented Jun 11, 2024

clyang82 left a comment

Choose a reason for hiding this comment

morvencao commented Jun 6, 2024 •

edited

Loading

skeeey Jun 6, 2024 •

edited

Loading

morvencao Jun 6, 2024 •

edited

Loading

morvencao commented Jun 7, 2024 •

edited

Loading