Skip to content

feat: update hub-agent to use custom cluster affinity plugin in scheduler when property checker is enabled #1221

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.

Sign up for GitHub

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

Merged
britaniar merged 1 commit into from
Nov 24, 2025
Merged

feat: update hub-agent to use custom cluster affinity plugin in scheduler when property checker is enabled #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions cmd/hubagent/options/azureoptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//
//Copyright (c) Microsoft Corporation.
//Licensed under the MIT license.

package options

import (
"flag"
)

// AzurePropertyCheckerOptions holds the options for the Azure property checker.
type AzurePropertyCheckerOptions struct {
// IsEnabled indicates whether the Azure property checker is enabled.
IsEnabled bool
// ComputeServiceAddressWithBasePath is the address of the Azure compute service with base path.
ComputeServiceAddressWithBasePath string
}

// NewAzurePropertyCheckerOptions creates a new AzurePropertyCheckerOptions with default values.
func NewAzurePropertyCheckerOptions() AzurePropertyCheckerOptions {
Copy link
Contributor

@jwtty jwtty Nov 24, 2025

Choose a reason for hiding this comment

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

ditto, do we want to return a pointer instead?

Copy link
Contributor

@zhiying-lin zhiying-lin Nov 25, 2025

Choose a reason for hiding this comment

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

i feel value is better here since the caller needs the value instead of the pointer.

return AzurePropertyCheckerOptions{
IsEnabled: false,
ComputeServiceAddressWithBasePath: "http://localhost:8421/compute",
}
}

// AddFlags adds flags for Azure Property Checker options to the given FlagSet.
func (o AzurePropertyCheckerOptions) AddFlags(flags *flag.FlagSet) {
Copy link
Contributor

@jwtty jwtty Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if we want to make this a pointer. cc @zhiying-lin like RatelimiterOpts: https://github.com/kubefleet-dev/kubefleet/blob/main/cmd/hubagent/options/ratelimit.go#L43

Copy link
Contributor

@zhiying-lin zhiying-lin Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

my bad, we have to use the pointer in this case, otherwise, when we check if opts.AzurePropertyCheckerOpts.IsEnabled {, it's using original value, as the flag is using the copy of the option.

flags.BoolVar(&o.IsEnabled, "azure-property-checker-enabled", o.IsEnabled, "Enable Azure property checker for validating Azure-specific cluster properties.")

Copy link
Contributor

@zhiying-lin zhiying-lin Nov 25, 2025

Choose a reason for hiding this comment

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

btw, here's the doc about receiver type, https://google.github.io/styleguide/go/decisions#receiver-type

flags.BoolVar(&o.IsEnabled, "azure-property-checker-enabled", o.IsEnabled, "Enable Azure property checker for validating Azure-specific cluster properties.")
flags.StringVar(&o.ComputeServiceAddressWithBasePath, "azure-compute-server-address", o.ComputeServiceAddressWithBasePath, "The address of the Azure compute service with base path.")
}
4 changes: 4 additions & 0 deletions cmd/hubagent/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ type Options struct {
ResourceSnapshotCreationMinimumInterval time.Duration
// ResourceChangesCollectionDuration is the duration for collecting resource changes into one snapshot.
ResourceChangesCollectionDuration time.Duration
// AzurePropertyCheckerOpts contains options for Azure property checker
AzurePropertyCheckerOpts AzurePropertyCheckerOptions
}

// NewOptions builds an empty options.
Expand All @@ -134,6 +136,7 @@ func NewOptions() *Options {
PprofPort: 6065,
ResourceSnapshotCreationMinimumInterval: 30 * time.Second,
ResourceChangesCollectionDuration: 15 * time.Second,
AzurePropertyCheckerOpts: NewAzurePropertyCheckerOptions(),
}
}

Expand Down Expand Up @@ -185,4 +188,5 @@ func (o *Options) AddFlags(flags *flag.FlagSet) {
flags.DurationVar(&o.ResourceChangesCollectionDuration, "resource-changes-collection-duration", 15*time.Second,
"The duration for collecting resource changes into one snapshot. The default is 15 seconds, which means that the controller will collect resource changes for 15 seconds before creating a resource snapshot.")
o.RateLimiterOpts.AddFlags(flags)
o.AzurePropertyCheckerOpts.AddFlags(flags)
}
22 changes: 20 additions & 2 deletions cmd/hubagent/workload/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
"go.goms.io/fleet/cmd/hubagent/options"
"go.goms.io/fleet/pkg/clients/azure/compute"
"go.goms.io/fleet/pkg/clients/httputil"
"go.goms.io/fleet/pkg/controllers/bindingwatcher"
"go.goms.io/fleet/pkg/controllers/clusterinventory/clusterprofile"
"go.goms.io/fleet/pkg/controllers/clusterresourceplacementeviction"
Expand All @@ -45,10 +47,12 @@ import (
"go.goms.io/fleet/pkg/controllers/schedulingpolicysnapshot"
"go.goms.io/fleet/pkg/controllers/updaterun"
"go.goms.io/fleet/pkg/controllers/workgenerator"
"go.goms.io/fleet/pkg/propertychecker/azure"
"go.goms.io/fleet/pkg/resourcewatcher"
"go.goms.io/fleet/pkg/scheduler"
"go.goms.io/fleet/pkg/scheduler/clustereligibilitychecker"
"go.goms.io/fleet/pkg/scheduler/framework"
"go.goms.io/fleet/pkg/scheduler/framework/plugins/clusteraffinity"
"go.goms.io/fleet/pkg/scheduler/profile"
"go.goms.io/fleet/pkg/scheduler/queue"
schedulerbindingwatcher "go.goms.io/fleet/pkg/scheduler/watchers/binding"
Expand Down Expand Up @@ -354,8 +358,22 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,

// Set up the scheduler
klog.Info("Setting up scheduler")
defaultProfile := profile.NewDefaultProfile()
defaultFramework := framework.NewFramework(defaultProfile, mgr)
schedulerProfile := profile.NewDefaultProfile()
if opts.AzurePropertyCheckerOpts.IsEnabled {
klog.Info("Azure property checker is enabled for cluster property validation")
client, err := compute.NewAttributeBasedVMSizeRecommenderClient(opts.AzurePropertyCheckerOpts.ComputeServiceAddressWithBasePath, httputil.DefaultClientForAzure)
if err != nil {
klog.ErrorS(err, "Unable to create Azure vm size recommender client")
return err
}
klog.Info("Setting up cluster affinity plugin with Azure property checker")
clusterAffinityPlugin := clusteraffinity.New(clusteraffinity.WithPropertyChecker(azure.NewPropertyChecker(*client)))
profileOpts := profile.Options{
ClusterAffinityPlugin: &clusterAffinityPlugin,
}
schedulerProfile = profile.NewProfile(profileOpts)
}
defaultFramework := framework.NewFramework(schedulerProfile, mgr)
defaultSchedulingQueue := queue.NewSimplePlacementSchedulingQueue(
queue.WithName(schedulerQueueName),
)
Expand Down
Loading