Repository: skypilot-org/skypilot
Pull request #5975 (2025-06-12 05:17:36)

[Bug] Fixed kubeconfig mounting in helm deployment
<!-- Describe the changes in this PR -->

close https://github.com/skypilot-org/skypilot/issues/5972

This is introduced by https://github.com/skypilot-org/skypilot/pull/5742, only `sshNodePools` enabled and both `kubeconfig` and `sshNodePools` enabled are tested in that PR. So unit test cased are added to avoid regression.

Also tested manually on real kubernetes cluster.

<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [x] Code formatting: install pre-commit (auto-check on commit) or `bash format.sh`
- [x] Any manual or new tests for this PR (please specify below)
- [ ] All smoke tests: `/smoke-test` (CI) or `pytest tests/test_smoke.py` (local)
- [ ] Relevant individual tests: `/smoke-test -k test_name` (CI) or `pytest tests/test_smoke.py::test_name` (local)
- [ ] Backward compatibility: `/quicktest-core` (CI) or `pytest tests/smoke_tests/test_backward_compat.py` (local)

<!-- CI commands (/-prefixed) can only be triggered by repo members -->



Patch:
diff --git a/charts/skypilot/.helmignore b/charts/skypilot/.helmignore
index e69de29bb..5941a6c0a 100644
--- a/charts/skypilot/.helmignore
+++ b/charts/skypilot/.helmignore
@@ -0,0 +1,1 @@
+unittests
diff --git a/charts/skypilot/templates/api-deployment.yaml b/charts/skypilot/templates/api-deployment.yaml
index 4ea457585..a7f3bd4de 100644
--- a/charts/skypilot/templates/api-deployment.yaml
+++ b/charts/skypilot/templates/api-deployment.yaml
@@ -184,7 +184,7 @@ spec:
           mountPath: /var/skypilot/kubeconfig
         {{- else }}
         - name: kube-config
-          mountPath: /root/.kube/config
+          mountPath: /root/.kube
         {{- end }}
         {{- end }}
         {{- if .Values.apiService.sshNodePools }}
diff --git a/sky/check.py b/sky/check.py
index 3bead5597..23684ed66 100644
--- a/sky/check.py
+++ b/sky/check.py
@@ -11,7 +11,6 @@ import colorama
 from sky import clouds as sky_clouds
 from sky import exceptions
 from sky import global_user_state
-from sky import sky_logging
 from sky import skypilot_config
 from sky.adaptors import cloudflare
 from sky.clouds import cloud as sky_cloud
@@ -24,30 +23,6 @@ from sky.utils import ux_utils
 CHECK_MARK_EMOJI = '\U00002714'  # Heavy check mark unicode
 PARTY_POPPER_EMOJI = '\U0001F389'  # Party popper unicode
 
-logger = sky_logging.init_logger(__name__)
-
-
-def _get_workspace_allowed_clouds(workspace: str) -> List[str]:
-    # Use allowed_clouds from config if it exists, otherwise check all
-    # clouds. Also validate names with get_cloud_tuple.
-    config_allowed_cloud_names = skypilot_config.get_nested(
-        ('allowed_clouds',),
-        [repr(c) for c in registry.CLOUD_REGISTRY.values()] + [cloudflare.NAME])
-    # filter out the clouds that are disabled in the workspace config
-    workspace_disabled_clouds = []
-    for cloud in config_allowed_cloud_names:
-        cloud_config = skypilot_config.get_workspace_cloud(cloud.lower(),
-                                                           workspace=workspace)
-        cloud_disabled = cloud_config.get('disabled', False)
-        if cloud_disabled:
-            workspace_disabled_clouds.append(cloud.lower())
-
-    config_allowed_cloud_names = [
-        c for c in config_allowed_cloud_names
-        if c.lower() not in workspace_disabled_clouds
-    ]
-    return config_allowed_cloud_names
-
 
 def check_capabilities(
     quiet: bool = False,
@@ -157,8 +132,6 @@ def check_capabilities(
             c for c in config_allowed_cloud_names
             if c not in workspace_disabled_clouds
         ]
-        global_user_state.set_allowed_clouds(
-            [c for c in config_allowed_cloud_names], current_workspace_name)
 
         # Use disallowed_cloud_names for logging the clouds that will be
         # disabled because they are not included in allowed_clouds in
@@ -368,22 +341,11 @@ def get_cached_enabled_clouds_or_refresh(
             raise_if_no_cloud_access is set to True.
     """
     active_workspace = skypilot_config.get_active_workspace()
-    allowed_clouds_changed = False
-    cached_allowed_clouds = global_user_state.get_allowed_clouds(
-        active_workspace)
-    skypilot_config_allowed_clouds = _get_workspace_allowed_clouds(
-        active_workspace)
-    if sorted(cached_allowed_clouds) != sorted(skypilot_config_allowed_clouds):
-        allowed_clouds_changed = True
-
     cached_enabled_clouds = global_user_state.get_cached_enabled_clouds(
         capability, active_workspace)
-    if not cached_enabled_clouds or allowed_clouds_changed:
+    if not cached_enabled_clouds:
         try:
             check_capability(capability, quiet=True, workspace=active_workspace)
-            if allowed_clouds_changed:
-                global_user_state.set_allowed_clouds(
-                    skypilot_config_allowed_clouds, active_workspace)
         except SystemExit:
             # If no cloud is enabled, check() will raise SystemExit.
             # Here we catch it and raise the exception later only if
diff --git a/sky/clouds/kubernetes.py b/sky/clouds/kubernetes.py
index d924cb700..d100de6ec 100644
--- a/sky/clouds/kubernetes.py
+++ b/sky/clouds/kubernetes.py
@@ -772,7 +772,7 @@ class Kubernetes(clouds.Cloud):
         """Checks if the user has access credentials to
         Kubernetes."""
         # Check for port forward dependencies
-        logger.debug(f'Checking compute credentials for {cls.canonical_name()}')
+        logger.info(f'Checking compute credentials for {cls.canonical_name()}')
         reasons = kubernetes_utils.check_port_forward_mode_dependencies(False)
         if reasons is not None:
             formatted = '\n'.join(
diff --git a/sky/global_user_state.py b/sky/global_user_state.py
index edb918df3..98a8dae58 100644
--- a/sky/global_user_state.py
+++ b/sky/global_user_state.py
@@ -45,7 +45,6 @@ if typing.TYPE_CHECKING:
 logger = sky_logging.init_logger(__name__)
 
 _ENABLED_CLOUDS_KEY_PREFIX = 'enabled_clouds_'
-_ALLOWED_CLOUDS_KEY_PREFIX = 'allowed_clouds_'
 
 _SQLALCHEMY_ENGINE: Optional[sqlalchemy.engine.Engine] = None
 _DB_INIT_LOCK = threading.Lock()
@@ -1088,43 +1087,6 @@ def _get_enabled_clouds_key(cloud_capability: 'cloud.CloudCapability',
     return _ENABLED_CLOUDS_KEY_PREFIX + workspace + '_' + cloud_capability.value
 
 
-@_init_db
-def get_allowed_clouds(workspace: str) -> List[str]:
-    assert _SQLALCHEMY_ENGINE is not None
-    with orm.Session(_SQLALCHEMY_ENGINE) as session:
-        row = session.query(config_table).filter_by(
-            key=_get_allowed_clouds_key(workspace)).first()
-    if row:
-        return json.loads(row.value)
-    return []
-
-
-@_init_db
-def set_allowed_clouds(allowed_clouds: List[str], workspace: str) -> None:
-    assert _SQLALCHEMY_ENGINE is not None
-    with orm.Session(_SQLALCHEMY_ENGINE) as session:
-        if (_SQLALCHEMY_ENGINE.dialect.name ==
-                db_utils.SQLAlchemyDialect.SQLITE.value):
-            insert_func = sqlite.insert
-        elif (_SQLALCHEMY_ENGINE.dialect.name ==
-              db_utils.SQLAlchemyDialect.POSTGRESQL.value):
-            insert_func = postgresql.insert
-        else:
-            raise ValueError('Unsupported database dialect')
-        insert_stmnt = insert_func(config_table).values(
-            key=_get_allowed_clouds_key(workspace),
-            value=json.dumps(allowed_clouds))
-        do_update_stmt = insert_stmnt.on_conflict_do_update(
-            index_elements=[config_table.c.key],
-            set_={config_table.c.value: json.dumps(allowed_clouds)})
-        session.execute(do_update_stmt)
-        session.commit()
-
-
-def _get_allowed_clouds_key(workspace: str) -> str:
-    return _ALLOWED_CLOUDS_KEY_PREFIX + workspace
-
-
 @_init_db
 def add_or_update_storage(storage_name: str,
                           storage_handle: 'Storage.StorageMetadata',


Test patch:
diff --git a/.github/workflows/helm-unittest.yaml b/.github/workflows/helm-unittest.yaml
new file mode 100644
index 000000000..3867941fd
--- /dev/null
+++ b/.github/workflows/helm-unittest.yaml
@@ -0,0 +1,38 @@
+name: Helm Unit Tests
+
+on:
+  push:
+    branches: [ main, master ]
+    paths:
+      - 'charts/**'
+  pull_request:
+    branches: [ main, master ]
+    paths:
+      - 'charts/**'
+  workflow_dispatch:
+
+jobs:
+  helm-unittest:
+    runs-on: ubuntu-latest
+    
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+      
+    - name: Set up Helm
+      uses: azure/setup-helm@v4
+      with:
+        version: '3.14.0'
+        
+    - name: Install helm unittest plugin
+      run: |
+        helm plugin install https://github.com/helm-unittest/helm-unittest.git
+        
+    - name: Update chart dependencies
+      run: |
+        cd charts/skypilot
+        helm dependency update
+        
+    - name: Run helm unit tests
+      run: |
+        helm unittest -f charts/skypilot/unittests/*.yaml charts/skypilot
diff --git a/charts/skypilot/unittests/deployment_test.yaml b/charts/skypilot/unittests/deployment_test.yaml
new file mode 100644
index 000000000..11f0e466a
--- /dev/null
+++ b/charts/skypilot/unittests/deployment_test.yaml
@@ -0,0 +1,53 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/helm-unittest/helm-unittest/main/schema/helm-testsuite.json
+suite: server_deployment_test
+templates:
+  - templates/api-deployment.yaml
+tests:
+  - it: should mount kubeconfig correctly when useKubeconfig is enabled
+    set:
+      kubernetesCredentials.useKubeconfig: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: kube-config
+            mountPath: /root/.kube
+      - contains:
+          path: spec.template.spec.volumes
+          content:
+            name: kube-config
+            secret:
+              secretName: kube-credentials
+
+  - it: should persist .kube directory when sshNodePool is enabled
+    set:
+      apiService.sshNodePools: test
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: state-volume
+            mountPath: /root/.kube
+            subPath: .kube
+
+  - it: should merge kubeconfig from .kube and configmap when useKubeconfig and sshNodePool are both enabled
+    set:
+      kubernetesCredentials.useKubeconfig: true
+      apiService.sshNodePools: test
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: kube-config
+            mountPath: /var/skypilot/kubeconfig
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: state-volume
+            mountPath: /root/.kube
+            subPath: .kube
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: KUBECONFIG
+            value: /root/.kube/config:/var/skypilot/kubeconfig/config
diff --git a/tests/unit_tests/test_resources.py b/tests/unit_tests/test_resources.py
index d637d30c9..c895edfcf 100644
--- a/tests/unit_tests/test_resources.py
+++ b/tests/unit_tests/test_resources.py
@@ -5,7 +5,6 @@ from unittest import mock
 
 import pytest
 
-from sky import check
 from sky import clouds
 from sky import global_user_state
 from sky import skypilot_config
@@ -104,10 +103,6 @@ def test_no_cloud_labels_resources():
     global_user_state.set_enabled_clouds(['aws', 'gcp'],
                                          sky_cloud.CloudCapability.COMPUTE,
                                          constants.SKYPILOT_DEFAULT_WORKSPACE)
-    global_user_state.set_allowed_clouds(
-        check._get_workspace_allowed_clouds(
-            constants.SKYPILOT_DEFAULT_WORKSPACE),
-        constants.SKYPILOT_DEFAULT_WORKSPACE)
     allowed_labels = {
         **GLOBAL_VALID_LABELS,
     }
@@ -123,10 +118,6 @@ def test_no_cloud_labels_resources_single_enabled_cloud():
     global_user_state.set_enabled_clouds(['aws'],
                                          sky_cloud.CloudCapability.COMPUTE,
                                          constants.SKYPILOT_DEFAULT_WORKSPACE)
-    global_user_state.set_allowed_clouds(
-        check._get_workspace_allowed_clouds(
-            constants.SKYPILOT_DEFAULT_WORKSPACE),
-        constants.SKYPILOT_DEFAULT_WORKSPACE)
     allowed_labels = {
         **GLOBAL_VALID_LABELS,
         'domain/key': 'value',  # Valid for AWS