Fix User Data Collection checkbox (opendatahub-io#404)

* fix default udc checkbox state * updated updateClusterSettings()
strangiato · Oct 18, 2023 · 1586f79 · 1586f79
1 parent 8916847
commit 1586f79
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 19 deletions.
diff --git a/backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts b/backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts
@@ -14,7 +14,7 @@ const DEFAULT_IDLENESS_CHECK_PERIOD = '1'; // 1 minute
 const DEFAULT_CLUSTER_SETTINGS: ClusterSettings = {
   pvcSize: DEFAULT_PVC_SIZE,
   cullerTimeout: DEFAULT_CULLER_TIMEOUT,
-  userTrackingEnabled: null,
+  userTrackingEnabled: false,
 };
 
 export const updateClusterSettings = async (
@@ -26,11 +26,11 @@ export const updateClusterSettings = async (
   const query = request.query as { [key: string]: string };
   const dashConfig = getDashboardConfig();
   try {
-    if (query.userTrackingEnabled !== 'null') {
-      await patchCM(fastify, segmentKeyCfg, {
-        data: { segmentKeyEnabled: query.userTrackingEnabled },
-      });
-    }
+    await patchCM(fastify, segmentKeyCfg, {
+      data: { segmentKeyEnabled: query.userTrackingEnabled },
+    }).catch((e) => {
+      fastify.log.error('Failed to update segment key enabled: ' + e.message);
+    });
     if (query.pvcSize && query.cullerTimeout) {
       if (dashConfig.spec?.notebookController?.enabled) {
         await setDashboardConfig(fastify, {
@@ -48,13 +48,13 @@ export const updateClusterSettings = async (
           isEnabled = false;
         }
         if (!isEnabled) {
-          await coreV1Api.deleteNamespacedConfigMap(nbcCfg, fastify.kube.namespace);
+          await coreV1Api.deleteNamespacedConfigMap(nbcCfg, fastify.kube.namespace).catch((e) => {
+            fastify.log.error('Failed to delete culler config: ') + e.message;
+          });
         } else {
-          try {
-            await patchCM(fastify, nbcCfg, {
-              data: { ENABLE_CULLING: String(isEnabled), CULL_IDLE_TIME: String(cullingTimeMin) },
-            });
-          } catch (e) {
+          await patchCM(fastify, nbcCfg, {
+            data: { ENABLE_CULLING: String(isEnabled), CULL_IDLE_TIME: String(cullingTimeMin) },
+          }).catch(async (e) => {
             if (e.statusCode === 404) {
               const cm: V1ConfigMap = {
                 apiVersion: 'v1',
@@ -69,19 +69,23 @@ export const updateClusterSettings = async (
                 },
               };
               await fastify.kube.coreV1Api.createNamespacedConfigMap(fastify.kube.namespace, cm);
+            } else {
+              fastify.log.error('Failed to patch culler config: ' + e.message);
             }
-          }
+          });
         }
       } else {
         patchCM(fastify, jupyterhubCfg, {
           data: {
             singleuser_pvc_size: `${query.pvcSize}Gi`,
             culler_timeout: query.cullerTimeout,
           },
+        }).catch((e) => {
+          fastify.log.error('Unable to patch JupyterHub config' + e.message);
         });
       }
     }
-    if (dashConfig.spec.notebookController.enabled) {
+    if (dashConfig.spec.notebookController?.enabled) {
       await rolloutDeployment(fastify, namespace, 'notebook-controller-deployment');
     } else {
       const jupyterhubCM = await coreV1Api.readNamespacedConfigMap(jupyterhubCfg, namespace);
@@ -94,10 +98,13 @@ export const updateClusterSettings = async (
     }
     return { success: true, error: null };
   } catch (e) {
+    fastify.log.error(
+      'Setting cluster settings error: ' + e.toString() + e.response?.body?.message,
+    );
     if (e.response?.statusCode !== 404) {
-      fastify.log.error('Setting cluster settings error: ' + e.toString() + e.respose.body.message);
       return { success: false, error: 'Unable to update cluster settings. ' + e.message };
     }
+    throw e;
   }
 };
 

diff --git a/backend/src/types.ts b/backend/src/types.ts
@@ -62,7 +62,7 @@ export type NotebookSize = {
 export type ClusterSettings = {
   pvcSize: number;
   cullerTimeout: number;
-  userTrackingEnabled: boolean | null;
+  userTrackingEnabled: boolean;
 };
 
 // Add a minimal QuickStart type here as there is no way to get types without pulling in frontend (React) modules

diff --git a/frontend/src/pages/clusterSettings/ClusterSettings.tsx b/frontend/src/pages/clusterSettings/ClusterSettings.tsx
@@ -52,7 +52,7 @@ const ClusterSettings: React.FC = () => {
   const [loadError, setLoadError] = React.useState<Error>();
   const [clusterSettings, setClusterSettings] = React.useState(DEFAULT_CONFIG);
   const [pvcSize, setPvcSize] = React.useState<number | string>(DEFAULT_PVC_SIZE);
-  const [userTrackingEnabled, setUserTrackingEnabled] = React.useState<boolean | null>(null);
+  const [userTrackingEnabled, setUserTrackingEnabled] = React.useState<boolean>(false);
   const [cullerTimeoutChecked, setCullerTimeoutChecked] =
     React.useState<string>(CULLER_TIMEOUT_UNLIMITED);
   const [cullerTimeout, setCullerTimeout] = React.useState<number>(DEFAULT_CULLER_TIMEOUT);

diff --git a/frontend/src/pages/clusterSettings/const.ts b/frontend/src/pages/clusterSettings/const.ts
@@ -15,5 +15,5 @@ export const DEFAULT_CULLER_TIMEOUT = 31536000; // 1 year as no culling
 export const DEFAULT_CONFIG: ClusterSettings = {
   pvcSize: DEFAULT_PVC_SIZE,
   cullerTimeout: DEFAULT_CULLER_TIMEOUT,
-  userTrackingEnabled: null,
+  userTrackingEnabled: false,
 };
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
@@ -75,7 +75,7 @@ export type NotebookSize = {
 };
 
 export type ClusterSettings = {
-  userTrackingEnabled: boolean | null;
+  userTrackingEnabled: boolean;
   pvcSize: number | string;
   cullerTimeout: number;
 };