Fix namespace arg for Python Helm SDK

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Bug fixes
 
 -   Fall back to client-side diff if server-side diff fails. (https://github.com/pulumi/pulumi-kubernetes/pull/685).
+-   Fix namespace arg for Python Helm SDK (https://github.com/pulumi/pulumi-kubernetes/pull/670).
 
 ## 0.25.4 (August 1, 2019)
 
@@ -37,7 +38,6 @@ that this change is not disruptive.
 
 -   Properly reference override values in Python Helm SDK (https://github.com/pulumi/pulumi-kubernetes/pull/676).
 -   Handle Output values in diffs. (https://github.com/pulumi/pulumi-kubernetes/pull/682).
--   Fall back to client-side diff if server-side diff fails. (https://github.com/pulumi/pulumi-kubernetes/pull/685).
 
 ## 0.25.3 (July 29, 2019)
 

pkg/gen/python-templates/helm/v2/helm.py

@@ -3,9 +3,10 @@
 
 import json
 import os.path
+import shutil
 import subprocess
-from tempfile import NamedTemporaryFile, TemporaryDirectory
-from typing import Callable, List, Optional, Tuple, Union
+from tempfile import mkdtemp, mkstemp
+from typing import Callable, List, Optional, TextIO, Tuple, Union
 
 import pulumi.runtime
 import yaml
@@ -294,52 +295,74 @@ def __init__(self,
         self.path = path
 
 
+def _run_helm_cmd(cmd: List[Union[str, bytes]]):
+    output = subprocess.run(
+        cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, check=True)
+    yaml_str: str = output.stdout
+    return yaml_str
+
+
+def _cleanup_temp_dir(all_config: Tuple[TextIO, Union[bytes, str], pulumi.Output]):
+    file, chart_dir, _ = all_config
+
+    file.close()
+    shutil.rmtree(chart_dir)
+
+
 def _parse_chart(all_config: Tuple[str, Union[ChartOpts, LocalChartOpts], pulumi.ResourceOptions]) -> pulumi.Output:
     release_name, config, opts = all_config
 
     # Create temporary directory and file to hold chart data and override values.
-    with NamedTemporaryFile() as overrides:
-        with TemporaryDirectory() as chart_dir:
-            if isinstance(config, ChartOpts):
-                chart_to_fetch = f'{config.repo}/{config.chart}' if config.repo else config.chart
-
-                # Configure fetch options.
-                fetch_opts_dict = {}
-                if config.fetch_opts is not None:
-                    fetch_opts_dict = {k: v for k, v in vars(config.fetch_opts).items() if v is not None}
-                fetch_opts_dict["destination"] = chart_dir
-                if config.version is not None:
-                    fetch_opts_dict["version"] = config.version
-                fetch_opts = FetchOpts(**fetch_opts_dict)
-
-                # Fetch the chart.
-                _fetch(chart_to_fetch, fetch_opts)
-                fetched_chart_name = os.listdir(chart_dir)[0]
-                chart = os.path.join(chart_dir, fetched_chart_name)
-            else:
-                chart = config.path
-
-            default_values = os.path.join(chart, 'values.yaml')
-
-            # Write overrides file.
-            vals = config.values if config.values is not None else {}
-            data = json.dumps(vals).encode('utf-8')
-            overrides.write(data)
-            overrides.flush()
-
-            namespace_arg = ['--namespace', config.namespace] if config.namespace else []
-
-            # Use 'helm template' to create a combined YAML manifest.
-            cmd = ['helm', 'template', chart, '--name', release_name,
-                   '--values', default_values, '--values', overrides.name]
-            cmd.extend(namespace_arg)
-
-            output = subprocess.run(
-                cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, check=True)
-            yaml_str: str = output.stdout
-
-            # Parse the manifest and create the specified resources.
-            return _parse_yaml_document(yaml.safe_load_all(yaml_str), opts, config.transformations)
+    # Note: We're intentionally using the lower-level APIs here because the async Outputs are being handled in
+    # a different scope, which was causing the temporary files/directory to be deleted before they were referenced
+    # in the Output handlers. We manually clean these up once we're done with another async handler that depends
+    # on the result of the operations.
+    overrides, overrides_filename = mkstemp()
+    chart_dir = mkdtemp()
+
+    if isinstance(config, ChartOpts):
+        chart_to_fetch = f'{config.repo}/{config.chart}' if config.repo else config.chart
+
+        # Configure fetch options.
+        fetch_opts_dict = {}
+        if config.fetch_opts is not None:
+            fetch_opts_dict = {k: v for k, v in vars(config.fetch_opts).items() if v is not None}
+        fetch_opts_dict["destination"] = chart_dir
+        if config.version is not None:
+            fetch_opts_dict["version"] = config.version
+        fetch_opts = FetchOpts(**fetch_opts_dict)
+
+        # Fetch the chart.
+        _fetch(chart_to_fetch, fetch_opts)
+        fetched_chart_name = os.listdir(chart_dir)[0]
+        chart = os.path.join(chart_dir, fetched_chart_name)
+    else:
+        chart = config.path
+
+    default_values = os.path.join(chart, 'values.yaml')
+
+    # Write overrides file.
+    vals = config.values if config.values is not None else {}
+    data = json.dumps(vals)
+    file = open(overrides, 'w')
+    file.write(data)
+    file.flush()
+
+    namespace_arg = ['--namespace', config.namespace] if config.namespace else []
+
+    # Use 'helm template' to create a combined YAML manifest.
+    cmd = ['helm', 'template', chart, '--name', release_name,
+           '--values', default_values, '--values', overrides_filename]
+    cmd.extend(namespace_arg)
+
+    chart_resources = pulumi.Output.from_input(cmd).apply(_run_helm_cmd)
+
+    # Parse the manifest and create the specified resources.
+    resources = chart_resources.apply(
+        lambda yaml_str: _parse_yaml_document(yaml.safe_load_all(yaml_str), opts, config.transformations))
+
+    pulumi.Output.all(file, chart_dir, resources).apply(_cleanup_temp_dir)
+    return resources
 
 
 def _fetch(chart: str, opts: FetchOpts) -> None:

sdk/python/pulumi_kubernetes/helm/v2/helm.py

@@ -3,9 +3,10 @@
 
 import json
 import os.path
+import shutil
 import subprocess
-from tempfile import NamedTemporaryFile, TemporaryDirectory
-from typing import Callable, List, Optional, Tuple, Union
+from tempfile import mkdtemp, mkstemp
+from typing import Callable, List, Optional, TextIO, Tuple, Union
 
 import pulumi.runtime
 import yaml
@@ -294,52 +295,74 @@ def __init__(self,
         self.path = path
 
 
+def _run_helm_cmd(cmd: List[Union[str, bytes]]):
+    output = subprocess.run(
+        cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, check=True)
+    yaml_str: str = output.stdout
+    return yaml_str
+
+
+def _cleanup_temp_dir(all_config: Tuple[TextIO, Union[bytes, str], pulumi.Output]):
+    file, chart_dir, _ = all_config
+
+    file.close()
+    shutil.rmtree(chart_dir)
+
+
 def _parse_chart(all_config: Tuple[str, Union[ChartOpts, LocalChartOpts], pulumi.ResourceOptions]) -> pulumi.Output:
     release_name, config, opts = all_config
 
     # Create temporary directory and file to hold chart data and override values.
-    with NamedTemporaryFile() as overrides:
-        with TemporaryDirectory() as chart_dir:
-            if isinstance(config, ChartOpts):
-                chart_to_fetch = f'{config.repo}/{config.chart}' if config.repo else config.chart
-
-                # Configure fetch options.
-                fetch_opts_dict = {}
-                if config.fetch_opts is not None:
-                    fetch_opts_dict = {k: v for k, v in vars(config.fetch_opts).items() if v is not None}
-                fetch_opts_dict["destination"] = chart_dir
-                if config.version is not None:
-                    fetch_opts_dict["version"] = config.version
-                fetch_opts = FetchOpts(**fetch_opts_dict)
-
-                # Fetch the chart.
-                _fetch(chart_to_fetch, fetch_opts)
-                fetched_chart_name = os.listdir(chart_dir)[0]
-                chart = os.path.join(chart_dir, fetched_chart_name)
-            else:
-                chart = config.path
-
-            default_values = os.path.join(chart, 'values.yaml')
-
-            # Write overrides file.
-            vals = config.values if config.values is not None else {}
-            data = json.dumps(vals).encode('utf-8')
-            overrides.write(data)
-            overrides.flush()
-
-            namespace_arg = ['--namespace', config.namespace] if config.namespace else []
-
-            # Use 'helm template' to create a combined YAML manifest.
-            cmd = ['helm', 'template', chart, '--name', release_name,
-                   '--values', default_values, '--values', overrides.name]
-            cmd.extend(namespace_arg)
-
-            output = subprocess.run(
-                cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, check=True)
-            yaml_str: str = output.stdout
-
-            # Parse the manifest and create the specified resources.
-            return _parse_yaml_document(yaml.safe_load_all(yaml_str), opts, config.transformations)
+    # Note: We're intentionally using the lower-level APIs here because the async Outputs are being handled in
+    # a different scope, which was causing the temporary files/directory to be deleted before they were referenced
+    # in the Output handlers. We manually clean these up once we're done with another async handler that depends
+    # on the result of the operations.
+    overrides, overrides_filename = mkstemp()
+    chart_dir = mkdtemp()
+
+    if isinstance(config, ChartOpts):
+        chart_to_fetch = f'{config.repo}/{config.chart}' if config.repo else config.chart
+
+        # Configure fetch options.
+        fetch_opts_dict = {}
+        if config.fetch_opts is not None:
+            fetch_opts_dict = {k: v for k, v in vars(config.fetch_opts).items() if v is not None}
+        fetch_opts_dict["destination"] = chart_dir
+        if config.version is not None:
+            fetch_opts_dict["version"] = config.version
+        fetch_opts = FetchOpts(**fetch_opts_dict)
+
+        # Fetch the chart.
+        _fetch(chart_to_fetch, fetch_opts)
+        fetched_chart_name = os.listdir(chart_dir)[0]
+        chart = os.path.join(chart_dir, fetched_chart_name)
+    else:
+        chart = config.path
+
+    default_values = os.path.join(chart, 'values.yaml')
+
+    # Write overrides file.
+    vals = config.values if config.values is not None else {}
+    data = json.dumps(vals)
+    file = open(overrides, 'w')
+    file.write(data)
+    file.flush()
+
+    namespace_arg = ['--namespace', config.namespace] if config.namespace else []
+
+    # Use 'helm template' to create a combined YAML manifest.
+    cmd = ['helm', 'template', chart, '--name', release_name,
+           '--values', default_values, '--values', overrides_filename]
+    cmd.extend(namespace_arg)
+
+    chart_resources = pulumi.Output.from_input(cmd).apply(_run_helm_cmd)
+
+    # Parse the manifest and create the specified resources.
+    resources = chart_resources.apply(
+        lambda yaml_str: _parse_yaml_document(yaml.safe_load_all(yaml_str), opts, config.transformations))
+
+    pulumi.Output.all(file, chart_dir, resources).apply(_cleanup_temp_dir)
+    return resources
 
 
 def _fetch(chart: str, opts: FetchOpts) -> None:

tests/examples/python/helm/__main__.py

@@ -11,12 +11,17 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+from pulumi_kubernetes.core.v1 import Namespace
 from pulumi_kubernetes.helm.v2 import Chart, ChartOpts
 
+namespace = Namespace("test")
+
 values = {"unbound": {"image": {"pullPolicy": "Always"}}}
 
-Chart("unbound", ChartOpts("stable/unbound", values=values))
+Chart("unbound", ChartOpts(
+    "stable/unbound", values=values, namespace=namespace.metadata["name"]))
 
 # Deploy a duplicate chart with a different resource prefix to verify that multiple instances of the Chart
 # can be managed in the same stack.
-Chart("unbound", ChartOpts("stable/unbound", resource_prefix="dup", values=values))
+Chart("unbound", ChartOpts(
+    "stable/unbound", resource_prefix="dup", values=values, namespace=namespace.metadata["name"]))

tests/examples/python/python_test.go

@@ -292,7 +292,7 @@ func TestHelm(t *testing.T) {
 		ExpectRefreshChanges: true, // PodDisruptionBudget status gets updated by the Deployment.
 		ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
 			assert.NotNil(t, stackInfo.Deployment)
-			assert.Equal(t, 12, len(stackInfo.Deployment.Resources))
+			assert.Equal(t, 13, len(stackInfo.Deployment.Resources))
 
 			sort.Slice(stackInfo.Deployment.Resources, func(i, j int) bool {
 				ri := stackInfo.Deployment.Resources[i]
@@ -318,11 +318,11 @@ func TestHelm(t *testing.T) {
 			assert.True(t, strings.HasPrefix(pullPolicy.(string), "Always"))
 
 			// Verify the provider resource.
-			provRes := stackInfo.Deployment.Resources[10]
+			provRes := stackInfo.Deployment.Resources[11]
 			assert.True(t, providers.IsProviderType(provRes.URN.Type()))
 
 			// Verify root resource.
-			stackRes := stackInfo.Deployment.Resources[11]
+			stackRes := stackInfo.Deployment.Resources[12]
 			assert.Equal(t, resource.RootStackType, stackRes.URN.Type())
 		},
 	})