address comments

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>

Author: britaniar

charts/hub-agent/templates/deployment.yaml

@@ -56,7 +56,7 @@ spec:
             - --resource-changes-collection-duration={{ .Values.resourceChangesCollectionDuration }}
             {{- if .Values.azurePropertyChecker.isEnabled }}
             - --azure-property-checker-enabled={{ .Values.azurePropertyChecker.isEnabled }}
-            - --azure-property-checker-config-file={{ .Values.azurePropertyChecker.configFilePath }}
+            - --azure-property-checker-config-file={{ .Values.azurePropertyChecker.computeServiceAddressWithBasePath }}
             {{- end }}
           ports:
             - name: metrics
@@ -86,18 +86,6 @@ spec:
                 fieldPath: metadata.namespace
           resources:
             {{- toYaml .Values.resources | nindent 12 }}
-          {{- if .Values.azurePropertyChecker.isEnabled }}
-          volumeMounts:
-            - name: property-checker-config
-              mountPath: /etc/kubernetes/checker
-              readOnly: true
-          {{- end }}
-      {{- if .Values.azurePropertyChecker.isEnabled }}
-      volumes:
-        - name: property-checker-config
-          secret:
-            secretName: property-config
-      {{- end }}
       {{- with .Values.affinity }}
       affinity:
         {{- toYaml . | nindent 8 }}

charts/hub-agent/values.yaml

@@ -62,6 +62,4 @@ MaxFleetSizeSupported: 100
 
 azurePropertyChecker:
   isEnabled: false
-  configFilePath: /etc/kubernetes/checker/config.json
-  config:
-    computeServiceAddressWithBasePath: "http://localhost:8421/compute"
+  computeServiceAddressWithBasePath: "http://localhost:8421/compute"

cmd/hubagent/main.go

@@ -91,9 +91,7 @@ func main() {
 
 	opts := options.NewOptions()
 	opts.AddFlags(flag.CommandLine)
-
-	azureOpts := options.NewAzurePropertyCheckerOptions()
-	azureOpts.AddFlags(flag.CommandLine)
+	opts.AzurePropertyCheckerOptions.AddFlags(flag.CommandLine)
 
 	flag.Parse()
 	defer handleExitFunc()
@@ -182,7 +180,7 @@ func main() {
 	}
 
 	ctx := ctrl.SetupSignalHandler()
-	if err := workload.SetupControllers(ctx, &wg, mgr, config, opts, azureOpts); err != nil {
+	if err := workload.SetupControllers(ctx, &wg, mgr, config, opts); err != nil {
 		klog.ErrorS(err, "unable to set up ready check")
 		exitWithErrorFunc()
 	}

cmd/hubagent/options/azureoptions.go

@@ -11,30 +11,30 @@ import (
 type AzurePropertyCheckerOptions struct {
 	// isEnabled indicates whether the Azure property checker is enabled.
 	isEnabled bool
-	// configFilePath is the path to the Azure property checker configuration file.
-	configFilePath string
+	// 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 {
 	return &AzurePropertyCheckerOptions{
-		isEnabled:      false,
-		configFilePath: "/etc/kubernetes/checker/config.json",
+		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) {
 	flags.BoolVar(&o.isEnabled, "azure-property-checker-enabled", o.isEnabled, "Enable Azure property checker for validating Azure-specific cluster properties.")
-	flags.StringVar(&o.configFilePath, "azure-property-checker-config-file", o.configFilePath, "Path to the Azure property checker configuration file.")
+	flags.StringVar(&o.computeServiceAddressWithBasePath, "azure-compute-server-address", o.computeServiceAddressWithBasePath, "The address of the Azure compute service with base path.")
 }
 
 // IsEnabled returns whether the Azure property checker is enabled.
 func (o *AzurePropertyCheckerOptions) IsEnabled() bool {
 	return o.isEnabled
 }
 
-// ConfigFilePath returns the path to the Azure property checker configuration file.
-func (o *AzurePropertyCheckerOptions) ConfigFilePath() string {
-	return o.configFilePath
+// ComputeServiceAddressWithBasePath returns the address of the Azure compute service with base path.
+func (o *AzurePropertyCheckerOptions) ComputeServiceAddressWithBasePath() string {
+	return o.computeServiceAddressWithBasePath
 }

cmd/hubagent/options/options.go

@@ -111,6 +111,8 @@ type Options struct {
 	ResourceSnapshotCreationMinimumInterval time.Duration
 	// ResourceChangesCollectionDuration is the duration for collecting resource changes into one snapshot.
 	ResourceChangesCollectionDuration time.Duration
+	// AzurePropertyCheckerOptions contains options for Azure property checker
+	AzurePropertyCheckerOptions *AzurePropertyCheckerOptions
 }
 
 // NewOptions builds an empty options.
@@ -133,6 +135,7 @@ func NewOptions() *Options {
 		PprofPort:                               6065,
 		ResourceSnapshotCreationMinimumInterval: 30 * time.Second,
 		ResourceChangesCollectionDuration:       15 * time.Second,
+		AzurePropertyCheckerOptions:             NewAzurePropertyCheckerOptions(),
 	}
 }
 

cmd/hubagent/workload/setup.go

@@ -134,7 +134,7 @@ var (
 )
 
 // SetupControllers set up the customized controllers we developed
-func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager, config *rest.Config, opts *options.Options, azureOpts *options.AzurePropertyCheckerOptions) error { //nolint:gocyclo
+func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager, config *rest.Config, opts *options.Options) error { //nolint:gocyclo
 	// TODO: Try to reduce the complexity of this last measured at 33 (failing at > 30) and remove the // nolint:gocyclo
 	dynamicClient, err := dynamic.NewForConfig(config)
 	if err != nil {
@@ -388,14 +388,9 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 		// Set up the scheduler
 		klog.Info("Setting up scheduler")
 		profileOpts := profile.Options{}
-		if azureOpts.IsEnabled() {
+		if opts.AzurePropertyCheckerOptions.IsEnabled() {
 			klog.Info("Azure property checker is enabled for cluster property validation")
-			propertyChckerConfig, err := azure.NewPropertyCheckerConfigFromFile(azureOpts.ConfigFilePath())
-			if err != nil {
-				klog.ErrorS(err, "unable to load azure property checker config")
-				return err
-			}
-			client, err := compute.NewAttributeBasedVMSizeRecommenderClient(propertyChckerConfig.ComputeServiceAddressWithBasePath, httputil.DefaultClientForAzure)
+			client, err := compute.NewAttributeBasedVMSizeRecommenderClient(opts.AzurePropertyCheckerOptions.ComputeServiceAddressWithBasePath(), httputil.DefaultClientForAzure)
 			if err != nil {
 				klog.ErrorS(err, "unable to create azure vm size recommender client")
 				return err

pkg/propertychecker/azure/config.go

@@ -112,11 +112,10 @@ var _ = BeforeSuite(func() {
 
 	By("Setup custom controllers")
 	opts := options.NewOptions()
-	azureOpts := options.NewAzurePropertyCheckerOptions()
 	opts.LeaderElection.LeaderElect = false
 	opts.EnableV1Alpha1APIs = true
 	opts.EnableV1Beta1APIs = false
-	err = workload.SetupControllers(ctx, nil, mgr, cfg, opts, azureOpts)
+	err = workload.SetupControllers(ctx, nil, mgr, cfg, opts)
 	Expect(err).Should(Succeed())
 
 	By("Start the controller manager")