fix: require instance type machine capabilities

crates/api-core/src/handlers/instance_type.rs

@@ -36,6 +36,24 @@ use crate::CarbideError;
 use crate::api::{Api, log_request_data};
 use crate::cfg::file::ComputeAllocationEnforcement;
 
+fn desired_capabilities_from_attributes(
+    attributes: Option<rpc::InstanceTypeAttributes>,
+) -> Result<Vec<InstanceTypeMachineCapabilityFilter>, Status> {
+    let desired_capabilities = attributes.unwrap_or_default().desired_capabilities;
+    if desired_capabilities.is_empty() {
+        return Err(CarbideError::InvalidArgument(
+            "at least one desired capability must be provided".to_string(),
+        )
+        .into());
+    }
+
+    desired_capabilities
+        .into_iter()
+        .map(InstanceTypeMachineCapabilityFilter::try_from)
+        .collect::<Result<Vec<_>, RpcDataConversionError>>()
+        .map_err(Into::into)
+}
+
 pub(crate) async fn create(
     api: &Api,
     request: Request<rpc::CreateInstanceTypeRequest>,
@@ -64,18 +82,7 @@ pub(crate) async fn create(
 
     metadata.validate(true).map_err(CarbideError::from)?;
 
-    // Prepare the capabilities list
-    let mut desired_capabilities = Vec::<InstanceTypeMachineCapabilityFilter>::new();
-
-    for cap in req
-        .instance_type_attributes
-        .unwrap_or(rpc::InstanceTypeAttributes {
-            ..Default::default()
-        })
-        .desired_capabilities
-    {
-        desired_capabilities.push(cap.try_into()?);
-    }
+    let desired_capabilities = desired_capabilities_from_attributes(req.instance_type_attributes)?;
 
     // Start a new transaction for a db write.
     let mut txn = api.txn_begin().await?;
@@ -274,18 +281,7 @@ pub(crate) async fn update(
 
     metadata.validate(true).map_err(CarbideError::from)?;
 
-    // Prepare the desired capabilities list
-    let mut desired_capabilities = Vec::<InstanceTypeMachineCapabilityFilter>::new();
-
-    for cap in req
-        .instance_type_attributes
-        .unwrap_or(rpc::InstanceTypeAttributes {
-            ..Default::default()
-        })
-        .desired_capabilities
-    {
-        desired_capabilities.push(cap.try_into()?);
-    }
+    let desired_capabilities = desired_capabilities_from_attributes(req.instance_type_attributes)?;
 
     // Start a new transaction for a db write.
     let mut txn = api.txn_begin().await?;

crates/api-core/src/tests/instance_type.rs

@@ -152,6 +152,84 @@ async fn test_instance_type_create(pool: sqlx::PgPool) -> Result<(), Box<dyn std
     Ok(())
 }
 
+#[crate::sqlx_test]
+async fn test_instance_type_create_and_update_require_desired_capabilities(pool: sqlx::PgPool) {
+    let env = create_test_env(pool).await;
+    let id = "requires_non_empty_capabilities";
+    let metadata = Some(rpc::forge::Metadata {
+        name: "requires non-empty capabilities".to_string(),
+        description: "".to_string(),
+        labels: vec![],
+    });
+
+    for (scenario, instance_type_attributes) in [
+        ("attributes omitted", None),
+        (
+            "desired capabilities empty",
+            Some(rpc::forge::InstanceTypeAttributes::default()),
+        ),
+    ] {
+        let status = env
+            .api
+            .create_instance_type(tonic::Request::new(rpc::forge::CreateInstanceTypeRequest {
+                id: Some(id.to_string()),
+                metadata: metadata.clone(),
+                instance_type_attributes,
+            }))
+            .await
+            .unwrap_err();
+
+        assert_eq!(status.code(), Code::InvalidArgument, "{scenario}");
+        assert_eq!(
+            status.message(),
+            "at least one desired capability must be provided",
+            "{scenario}"
+        );
+    }
+
+    let valid_attributes = Some(rpc::forge::InstanceTypeAttributes {
+        desired_capabilities: vec![rpc::forge::InstanceTypeMachineCapabilityFilterAttributes {
+            capability_type: rpc::forge::MachineCapabilityType::CapTypeCpu.into(),
+            name: Some("pentium 4 HT".to_string()),
+            ..Default::default()
+        }],
+    });
+    env.api
+        .create_instance_type(tonic::Request::new(rpc::forge::CreateInstanceTypeRequest {
+            id: Some(id.to_string()),
+            metadata: metadata.clone(),
+            instance_type_attributes: valid_attributes,
+        }))
+        .await
+        .unwrap();
+
+    for (scenario, instance_type_attributes) in [
+        ("attributes omitted", None),
+        (
+            "desired capabilities empty",
+            Some(rpc::forge::InstanceTypeAttributes::default()),
+        ),
+    ] {
+        let status = env
+            .api
+            .update_instance_type(tonic::Request::new(rpc::forge::UpdateInstanceTypeRequest {
+                id: id.to_string(),
+                metadata: metadata.clone(),
+                instance_type_attributes,
+                if_version_match: None,
+            }))
+            .await
+            .unwrap_err();
+
+        assert_eq!(status.code(), Code::InvalidArgument, "{scenario}");
+        assert_eq!(
+            status.message(),
+            "at least one desired capability must be provided",
+            "{scenario}"
+        );
+    }
+}
+
 #[crate::sqlx_test]
 async fn test_instance_type_update(pool: sqlx::PgPool) -> Result<(), Box<dyn std::error::Error>> {
     let env = create_test_env(pool).await;

docs/observability/core_metrics.md

@@ -123,6 +123,7 @@ This file contains a list of metrics exported by NVIDIA Infra Controller (NICo).
 <tr><td>carbide_site_explorer_created_power_shelves_count</td><td>gauge</td><td>The amount of Power Shelves that had been created by Site Explorer after being identified</td></tr>
 <tr><td>carbide_site_explorer_enabled</td><td>gauge</td><td>Whether site-explorer is enabled (1) or paused (0)</td></tr>
 <tr><td>carbide_site_explorer_iteration_latency_milliseconds</td><td>histogram</td><td>The time it took to perform one site explorer iteration</td></tr>
+<tr><td>carbide_site_explorer_last_run_status</td><td>gauge</td><td>The status of the latest Site Explorer run</td></tr>
 <tr><td>carbide_site_explorer_phase_latency_milliseconds</td><td>histogram</td><td>The time it took to perform one site explorer iteration phase</td></tr>
 <tr><td>carbide_site_explorer_update_explored_endpoints_count</td><td>gauge</td><td>Counts from the last update_explored_endpoints phase by kind</td></tr>
 <tr><td>carbide_switches_enqueuer_iteration_latency_milliseconds</td><td>histogram</td><td>The overall time it took to enqueue state handling tasks for all carbide_switches in the system</td></tr>

rest-api/api/pkg/api/handler/instancetype_test.go

@@ -205,19 +205,38 @@ func TestCreateInstanceTypeHandler_Handle(t *testing.T) {
 		},
 	}
 
-	itcrValidWithoutMachineCapabilities := &model.APIInstanceTypeCreateRequest{
+	itcrInvalidMissingMachineCapabilities := &model.APIInstanceTypeCreateRequest{
 		Name:        "x2.large.missing.mc",
 		Description: sutil.GetPtr("Test Description"),
 		SiteID:      st.ID.String(),
 	}
 
+	itcrInvalidEmptyMachineCapabilities := &model.APIInstanceTypeCreateRequest{
+		Name:                "x2.large.empty.mc",
+		Description:         sutil.GetPtr("Test Description"),
+		SiteID:              st.ID.String(),
+		MachineCapabilities: []model.APIMachineCapability{},
+	}
+
 	itcrInvalid1 := &model.APIInstanceTypeCreateRequest{
 		Name: "x2.large",
+		MachineCapabilities: []model.APIMachineCapability{
+			{
+				Type: cdbm.MachineCapabilityTypeCPU,
+				Name: "Intel Xeon E5-2650 v2",
+			},
+		},
 	}
 
 	itcrInvalid2 := &model.APIInstanceTypeCreateRequest{
 		Name:   "x2.large",
 		SiteID: uuid.New().String(),
+		MachineCapabilities: []model.APIMachineCapability{
+			{
+				Type: cdbm.MachineCapabilityTypeCPU,
+				Name: "Intel Xeon E5-2650 v2",
+			},
+		},
 	}
 
 	common.TestBuildInstanceType(t, dbSession, "test-it-name-1", nil, st, map[string]string{
@@ -234,6 +253,12 @@ func TestCreateInstanceTypeHandler_Handle(t *testing.T) {
 		Description:           sutil.GetPtr("Test Description"),
 		SiteID:                st.ID.String(),
 		ControllerMachineType: sutil.GetPtr("intel_xeon_e5_2650v2"),
+		MachineCapabilities: []model.APIMachineCapability{
+			{
+				Type: cdbm.MachineCapabilityTypeCPU,
+				Name: "Intel Xeon E5-2650 v2",
+			},
+		},
 	}
 
 	cfg := common.GetTestConfig()
@@ -339,7 +364,12 @@ func TestCreateInstanceTypeHandler_Handle(t *testing.T) {
 						"description": "Test x9001 Instance Type ",
 					},
 					ControllerMachineType: sutil.GetPtr("intel_goku_e9001_dbzv2"),
-					MachineCapabilities:   []model.APIMachineCapability{},
+					MachineCapabilities: []model.APIMachineCapability{
+						{
+							Type: cdbm.MachineCapabilityTypeCPU,
+							Name: "Intel Goku E9001",
+						},
+					},
 				},
 			},
 			wantErr:                false,
@@ -360,12 +390,18 @@ func TestCreateInstanceTypeHandler_Handle(t *testing.T) {
 					Description:           sutil.GetPtr("Test Description"),
 					SiteID:                st.ID.String(),
 					ControllerMachineType: sutil.GetPtr("intel_goku_e9001_dbzv2"),
-					MachineCapabilities:   []model.APIMachineCapability{},
+					MachineCapabilities: []model.APIMachineCapability{
+						{
+							Type: cdbm.MachineCapabilityTypeCPU,
+							Name: "Intel Goku E9001",
+						},
+					},
 				},
 			},
-			wantErr:                false,
-			expectedResourcesCount: 1,
-			respCode:               http.StatusCreated,
+			wantErr:                     false,
+			expectedResourcesCount:      1,
+			expectedMachineCapabilities: 1,
+			respCode:                    http.StatusCreated,
 		},
 		{
 			name: "test create Instance Type API endpoint with valid data",
@@ -385,20 +421,32 @@ func TestCreateInstanceTypeHandler_Handle(t *testing.T) {
 			verifyChildSpanner:          true,
 		},
 		{
-			name: "test create Instance Type API endpoint with valid data but no machine capabilities",
+			name: "test create Instance Type API endpoint rejects missing machine capabilities",
 			fields: fields{
 				dbSession: dbSession,
 				tc:        &tmocks.Client{},
 				scp:       scp,
 				cfg:       cfg,
 			},
 			args: args{
-				reqData: itcrValidWithoutMachineCapabilities,
+				reqData: itcrInvalidMissingMachineCapabilities,
 			},
-			wantErr:                     false,
-			respCode:                    http.StatusCreated,
-			expectedResourcesCount:      1,
-			expectedMachineCapabilities: 0,
+			wantErr:  false,
+			respCode: http.StatusBadRequest,
+		},
+		{
+			name: "test create Instance Type API endpoint rejects empty machine capabilities",
+			fields: fields{
+				dbSession: dbSession,
+				tc:        &tmocks.Client{},
+				scp:       scp,
+				cfg:       cfg,
+			},
+			args: args{
+				reqData: itcrInvalidEmptyMachineCapabilities,
+			},
+			wantErr:  false,
+			respCode: http.StatusBadRequest,
 		},
 		{
 			name: "error create Instance Type API endpoint with name clash",
@@ -2115,6 +2163,24 @@ func TestUpdateInstanceTypeHandler_Handle(t *testing.T) {
 			},
 			wantRespCode: http.StatusOK,
 		},
+		{
+			name: "test Instance Type update rejects empty machine capabilities",
+			fields: fields{
+				dbSession: dbSession,
+				tc:        &tmocks.Client{},
+				scp:       scp,
+				cfg:       cfg,
+			},
+			args: args{
+				user:           ipu,
+				org:            org,
+				instanceTypeID: it2.ID,
+				reqData: &model.APIInstanceTypeUpdateRequest{
+					MachineCapabilities: []model.APIMachineCapability{},
+				},
+			},
+			wantRespCode: http.StatusBadRequest,
+		},
 		{
 			name: "test Instance Type update fail with new capability of a bad type",
 			fields: fields{

rest-api/api/pkg/api/model/instancetype.go

@@ -43,7 +43,8 @@ func (itcr *APIInstanceTypeCreateRequest) Validate() error {
 			validation.Required.Error(validationErrorValueRequired),
 			validationis.UUID.Error(validationErrorInvalidUUID)),
 		validation.Field(&itcr.Labels, validation.By(util.ValidateLabels)),
-		validation.Field(&itcr.MachineCapabilities),
+		validation.Field(&itcr.MachineCapabilities,
+			validation.Required.Error(validationErrorValueRequired)),
 	)
 }
 
@@ -87,7 +88,9 @@ func (itur *APIInstanceTypeUpdateRequest) Validate() error {
 			validation.When(itur.Name != nil, validation.By(util.ValidateNameCharacters)),
 			validation.When(itur.Name != nil, validation.Length(2, 256).Error(validationErrorStringLength))),
 		validation.Field(&itur.Labels, validation.By(util.ValidateLabels)),
-		validation.Field(&itur.MachineCapabilities),
+		validation.Field(&itur.MachineCapabilities,
+			validation.When(itur.MachineCapabilities != nil,
+				validation.Required.Error(validationErrorValueRequired))),
 	)
 }