diff --git a/cmd/agent/app/agent.go b/cmd/agent/app/agent.go index fe9bf41a2..69dd81474 100644 --- a/cmd/agent/app/agent.go +++ b/cmd/agent/app/agent.go @@ -38,11 +38,23 @@ func NewAgentCommand(ctx context.Context) *cobra.Command { cmd := &cobra.Command{ Use: "karmada-agent", Long: `The karmada agent runs the cluster registration agent`, - Run: func(cmd *cobra.Command, args []string) { - if err := run(ctx, karmadaConfig, opts); err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) + RunE: func(cmd *cobra.Command, args []string) error { + // validate options + if errs := opts.Validate(); len(errs) != 0 { + return errs.ToAggregate() } + if err := run(ctx, karmadaConfig, opts); err != nil { + return err + } + return nil + }, + Args: func(cmd *cobra.Command, args []string) error { + for _, arg := range args { + if len(arg) > 0 { + return fmt.Errorf("%q does not take any arguments, got %q", cmd.CommandPath(), args) + } + } + return nil }, } diff --git a/cmd/agent/app/options/validation.go b/cmd/agent/app/options/validation.go new file mode 100644 index 000000000..444a52254 --- /dev/null +++ b/cmd/agent/app/options/validation.go @@ -0,0 +1,28 @@ +package options + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// Validate checks Options and return a slice of found errs. +func (o *Options) Validate() field.ErrorList { + errs := field.ErrorList{} + + newPath := field.NewPath("Options") + if len(o.ClusterName) == 0 { + errs = append(errs, field.Invalid(newPath.Child("ClusterName"), o.ClusterName, "clusterName cannot be empty")) + } + if o.ClusterStatusUpdateFrequency.Duration < 0 { + errs = append(errs, field.Invalid(newPath.Child("ClusterStatusUpdateFrequency"), o.ClusterStatusUpdateFrequency, "must be greater than or equal to 0")) + } + + if o.ClusterLeaseDuration.Duration < 0 { + errs = append(errs, field.Invalid(newPath.Child("ClusterLeaseDuration"), o.ClusterLeaseDuration, "must be greater than or equal to 0")) + } + + if o.ClusterLeaseRenewIntervalFraction <= 0 { + errs = append(errs, field.Invalid(newPath.Child("ClusterLeaseRenewIntervalFraction"), o.ClusterLeaseRenewIntervalFraction, "must be greater than 0")) + } + + return errs +} diff --git a/cmd/agent/app/options/validation_test.go b/cmd/agent/app/options/validation_test.go new file mode 100644 index 000000000..ae837cbdb --- /dev/null +++ b/cmd/agent/app/options/validation_test.go @@ -0,0 +1,106 @@ +package options + +import ( + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + componentbaseconfig "k8s.io/component-base/config" +) + +func TestValidateKarmadaAgentConfiguration(t *testing.T) { + successCases := []Options{ + { + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + }, + ClusterName: "demo", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: 40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0.25, + }, + { + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: true, + }, + ClusterName: "demo", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: 40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0.25, + }, + } + + for _, successCase := range successCases { + if errs := successCase.Validate(); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + } + + newPath := field.NewPath("Options") + testCases := map[string]struct { + opt Options + expectedErrs field.ErrorList + }{ + "invalid ClusterName": { + opt: Options{ + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + }, + ClusterName: "", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: 40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0.25, + }, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterName"), "", "clusterName cannot be empty")}, + }, + "invalid ClusterStatusUpdateFrequency": { + opt: Options{ + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + }, + ClusterName: "demo", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: -10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: 40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0.25, + }, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterStatusUpdateFrequency"), metav1.Duration{Duration: -10 * time.Second}, "must be greater than or equal to 0")}, + }, + "invalid ClusterLeaseDuration": { + opt: Options{ + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + }, + ClusterName: "demo", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: -40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0.25, + }, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterLeaseDuration"), metav1.Duration{Duration: -40 * time.Second}, "must be greater than or equal to 0")}, + }, + "invalid ClusterLeaseRenewIntervalFraction": { + opt: Options{ + LeaderElection: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + }, + ClusterName: "demo", + ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second}, + ClusterLeaseDuration: metav1.Duration{Duration: 40 * time.Second}, + ClusterLeaseRenewIntervalFraction: 0, + }, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterLeaseRenewIntervalFraction"), 0, "must be greater than 0")}, + }, + } + + for _, testCase := range testCases { + errs := testCase.opt.Validate() + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Fatalf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } + } + } +} diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 817cd4eea..e61cdf23a 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -1,11 +1,12 @@ package main import ( + "os" + _ "sigs.k8s.io/controller-runtime/pkg/metrics" apiserver "k8s.io/apiserver/pkg/server" "k8s.io/component-base/logs" - "k8s.io/klog/v2" "github.com/karmada-io/karmada/cmd/agent/app" ) @@ -17,6 +18,6 @@ func main() { ctx := apiserver.SetupSignalContext() if err := app.NewAgentCommand(ctx).Execute(); err != nil { - klog.Fatal(err.Error()) + os.Exit(1) } }