remove redundant flag; imporve error handling

cmd/kip/flags.go

@@ -23,7 +23,6 @@ type ServerConfig struct {
 	NetworkAgentSecret     string
 	NetworkAgentKubeConfig string
 	ClusterDNS             string
-	OverrideInstanceData   bool
 	InstanceDataPath       string
 }
 
@@ -33,7 +32,6 @@ func (c *ServerConfig) FlagSet() *pflag.FlagSet {
 	flags.StringVar(&c.NetworkAgentSecret, "network-agent-secret", c.NetworkAgentSecret, "Service account secret for the cell network agent, in the form of <namespace>/<name>")
 	flags.StringVar(&c.NetworkAgentKubeConfig, "network-agent-kubeconfig", c.NetworkAgentKubeConfig, "Network agent kubeconfig file, mutually exclusive with --network-agent-secret")
 	flags.StringVar(&c.ClusterDNS, "cluster-dns", c.ClusterDNS, "Default cluster DNS server to use; if not specified, the kube-system/kube-dns service IP will be used")
-	flags.BoolVar(&c.OverrideInstanceData, "override-instance-data", c.OverrideInstanceData, "set this to true to specify custom instance-data path")
-	flags.StringVar(&c.InstanceDataPath, "instance-data-path", c.InstanceDataPath, "instance-data path; ignored if override-instance-data isn't set to true")
+	flags.StringVar(&c.InstanceDataPath, "instance-data-path", c.InstanceDataPath, "instance-data path; instanceselector will try to load json file in this path and use it as pricing data")
 	return flags
 }

cmd/kip/main.go

@@ -104,10 +104,6 @@ func main() {
 					// Unable to continue without network agent kubeconfig.
 					log.G(ctx).Fatalf("%v", err)
 				}
-				instanceDataPath := ""
-				if serverConfig.OverrideInstanceData {
-					instanceDataPath = serverConfig.InstanceDataPath
-				}
 				return server.NewInstanceProvider(
 					cfg.ConfigPath,
 					cfg.NodeName,
@@ -119,7 +115,7 @@ func main() {
 					cfg.ResourceManager,
 					kubeConfig,
 					networkAgentKubeConfig,
-					instanceDataPath,
+					serverConfig.InstanceDataPath,
 					ctx.Done(),
 				)
 			}),

pkg/server/server.go

@@ -253,7 +253,9 @@ func NewInstanceProvider(configFilePath, nodeName, internalIP, clusterDNS, clust
 	klog.V(5).Infof("setting up instance selector")
 	if instanceDataPath != "" {
 		_, err = os.Stat(instanceDataPath)
-		return nil, fmt.Errorf("cannot load custom instance data from path %s: %v", instanceDataPath, err)
+		if err != nil {
+			return nil, fmt.Errorf("cannot load custom instance data from path %s: %v", instanceDataPath, err)
+		}
 	}
 	err = instanceselector.Setup(
 		cloudClient.GetAttributes().Provider,

pkg/util/instanceselector/instanceselector.go

@@ -146,6 +146,8 @@ func getSelectorData(data, regionOrZone, filepath string) ([]InstanceData, error
 		// If loading from file is successful, return data from there,
 		// if not, fallback to data from instanceselector package
 		return fileData, nil
+	} else {
+		klog.Warningf("failed to load instance data from path %s: %v , falling back to data baked in binary", filepath, err)
 	}
 	err = json.Unmarshal([]byte(data), &d)
 	if err != nil {

pkg/util/instanceselector/instanceselector_test.go

@@ -464,4 +464,4 @@ func TestGetSelectorData(t *testing.T)  {
 	// happy path: loading from local file failed, but fallback to local variable worked
 	_, err = getSelectorData(awsInstanceJson, "af-south-1", invalidPath)
 	assert.NoError(t, err)
-}
+}

pkg/util/instanceselector/testdata/invalid_test_data.json

@@ -12,4 +12,4 @@
       "gpu": 0
     }
   ],
-}
+}

pkg/util/instanceselector/testdata/valid_test_data.json

@@ -12,4 +12,4 @@
       "gpu": 0
     }
   ]
-}
+}