cdk8s-team / cdk8s

Define Kubernetes native apps and abstractions using object-oriented programming
https://cdk8s.io
Apache License 2.0
4.36k stars 294 forks source link

Multiple Imports Fail To Synth #273

Closed gabe-l-hart closed 4 years ago

gabe-l-hart commented 4 years ago

Description of the bug:

When working on a project with multiple imports (such as the example for CRDs), the cdk8s import function works smoothly to generate the full set of definitions, but cdk8s synth (or invoking app.synth() from code) results in a jsii.errors.JavaScriptError due to conflicting definitions of the generated package in nodejs. I've observed this both in the crd sample and in an internal project, so the behavior seems to be consistent for any project using multiple imports.

There seem to also be some secondary bugs in the crd example. The paths generated via cdk8s imports did not match the coded import paths in main.py. I needed to change them from

from imports import jenkins
from imports import clusterinstallation

to

from imports.jenkins.io import jenkins
from imports.mattermost.com import clusterinstallation

Reproduction Steps:

  1. cd examples/python/crd
  2. cdk8s import
  3. Make the above changes to main.py
  4. cdk8s synth

Error Log:

jsii.errors.JavaScriptError: 
  Error: Type 'generated.ClusterInstallation' not found
Full Error ``` jsii.errors.JavaScriptError: Error: Type 'generated.ClusterInstallation' not found at Kernel._typeInfoForFqn (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:8200:19) at Kernel._findCtor (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7898:31) at Kernel._create (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7920:33) at Kernel.create (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7666:21) at KernelHost.processRequest (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7446:28) at KernelHost.run (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7384:14) at Immediate._onImmediate (/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7387:37) at processImmediate (internal/timers.js:439:21) The above exception was the direct cause of the following exception: Traceback (most recent call last): File "./main.py", line 49, in MyChart(app, "crd-python") File "/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_runtime.py", line 66, in __call__ inst = super().__call__(*args, **kwargs) File "./main.py", line 44, in __init__ mattermost_license_secret="mattermost-license" File "/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_runtime.py", line 66, in __call__ inst = super().__call__(*args, **kwargs) File "/Users/ghart/Projects/github/cdk8s/examples/python/crd/imports/mattermost/com/clusterinstallation/__init__.py", line 33, in __init__ jsii.create(ClusterInstallation, self, [scope, name, options]) File "/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 229, in create interfaces=[iface.__jsii_type__ for iface in getattr(klass, "__jsii_ifaces__", [])], File "/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 333, in create return self._process.send(request, CreateResponse) File "/Users/ghart/.local/share/virtualenvs/crd-3rAX9go0/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 318, in send raise JSIIError(resp.error) from JavaScriptError(resp.stack) jsii.errors.JSIIError: Type 'generated.ClusterInstallation' not found Error: command "pipenv run ./main.py " at /Users/ghart/Projects/github/cdk8s/examples/python/crd returned a non-zero exit code 1 at ChildProcess. (/usr/local/Cellar/cdk8s/0.25.0/libexec/lib/node_modules/cdk8s-cli/lib/util.js:24:27) at Object.onceWrapper (events.js:300:26) at ChildProcess.emit (events.js:210:5) at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12) ```

Environment:

For my internal project, the build environment is configured via docker. Here is a Dockerfile that will reproduce the issue with the crd example:

FROM ubuntu:bionic

RUN apt update -y
RUN apt install -y curl git python3-pip python3-venv zlib1g-dev libssl-dev \
    && curl -sL https://deb.nodesource.com/setup_12.x | bash - \
    && apt install -y nodejs \
    && npm install -g cdk8s-cli \
    && pip3 install pipenv \
    && curl https://pyenv.run | bash

ENV LC_ALL=C.UTF-8
ENV LANG=C.UTF-8
ENV PATH=$PATH:/root/.pyenv/bin

RUN mkdir /src \
    && cd /src \
    && git clone https://github.com/awslabs/cdk8s.git \
    && cd /src/cdk8s/examples/python/crd \
    && pyenv install 3.7.8 \
    && pipenv install \
    && pipenv install constructs cdk8s \
    && cdk8s import \
    && sed -i 's,from imports import clusterinstallation,from imports.mattermost.com import clusterinstallation,' main.py \
    && sed -i 's,from imports import jenkins,from imports.jenkins.io import jenkins,' main.py

# This is where the error occurs
RUN cd /src/cdk8s/examples/python/crd \
    && cdk8s synth

Other:

I've been trying to learn the interplay between cdk8s, jsii, and (just discovered) jsii-srcmak. From what I can tell, the issue is arising because the name field in the package.json inside of the generated nodejs package is generated for all packages generated with jsii-srcmak here. My best guess so far is that the generated node package is conflicting and whichever gets imported first in python is the one that gets correctly registered with the node process, and all others that are imported from python after that are not correctly imported.


This is :bug: Bug Report

gabe-l-hart commented 4 years ago

Some further investigation is showing that this is likely caused due to the name collision with generated triggering jsii to fail to properly install the second instance. This section of the jsii kernel seems to be performing the untar operation directly into the temporary installDir, but silently ignoring errors. I suspect that the second attempt to untar the package named generated is silently failing. When I look at the contents of the temporary installDir after a failed execution, I see the fully expanded package from the first import, but not the second.

gabe-l-hart commented 4 years ago

More investigation:

I was able to get my repro case "working" by doing the following steps (not a fix, but hopefully another helpful pointer towards one):

  1. For one of the generated jsii tarballs, untar it in a temporary directory
  2. Replace all mentions of the string generated with generatedFoo in the .jsii file and package.json file
  3. Re-tar that modified package directory as generatedFoo@0.0.0.jsii.tgz and place it alongside generated@0.0.0.jsii.tgz
  4. In the generated python library, modify the top-level __init__.py and the _jsii/__init__.py with the same generated -> generatedFoo replacement

I will look into scripting these steps for the time being to unblock my project, but it feels like the fix is probably to update jsii-srcmak to allow for a flexible naming scheme and to generate some sort of unique name for each import in the cdk8s usage.

gabe-l-hart commented 4 years ago

As a temporary workaround, I'm going to use the following script to refactor the generated imports after the initial generation pass:

#!/usr/bin/env bash

# This script fixes an problem caused by cdk8s + jsii + jsii-srcmak when using
# multiple imports.
#
# Open Issue: https://github.com/awslabs/cdk8s/issues/273

import_path=""
while [[ $# > 0 ]]
do
  key="$1"
  case $key in
    -i|--import-path)
      shift
      import_path=$(cd $1 && pwd)
    ;;
    *)
      echo "Unrecognized option [$key]"
      exit 1
    ;;
  esac
  shift
done

if [ "$import_path" == "" ]
then
  echo "Missing required arg [--import-path]"
  exit 1
fi

# Compute a unique name hash for this import based on the content of the
# top-level init (which will be unique)
suffix="$(md5sum $import_path/__init__.py | cut -c1-8)"
replace="generated$suffix"
echo "Replace: [$replace]"

# Function to replace 'generated' everywhere in the given file
function do_replace {
  fname=$1
  sed -i "s,generated,$replace,g" $fname
}

# Replace in each of the __init__ files
do_replace $import_path/__init__.py
do_replace $import_path/_jsii/__init__.py

# Use a temporary directory to fix the content of the tar file
temp_dir="$(mktemp -d -t cdk8s-fix-XXXXXXXXXX)"
echo "Using temp_dir=$temp_dir"

# Untar the packed content into the temp dir
cd "$temp_dir"
tar -xzvf "$import_path/_jsii/generated@0.0.0.jsii.tgz"

# Replace in the .jsii file and package.json
do_replace package/.jsii
do_replace package/package.json

# Re-tar the file with the new name and put it back into the import dir
new_tar_name="$replace@0.0.0.jsii.tgz"
tar -czvf "$new_tar_name" package
mv "$new_tar_name" "$import_path/_jsii/"

# Remove the old tar file and the temp dir
cd -
rm "$import_path/_jsii/generated@0.0.0.jsii.tgz"
rm -rf $temp_dir
eladb commented 4 years ago

@gabe-l-hart thanks so much for the great report and analysis. Sounds like indeed the issue is in jsii-srcmak. ~I am transferring the issue to srcmak for follow up.~ (since srcmak is in a different repo, we will continue the follow up here).

eladb commented 4 years ago

@campionfellin would you like to take a look at this?

campionfellin commented 4 years ago

I'll take a look!

campionfellin commented 4 years ago

Indeed, it does look like this line: jsii-srcmak/src/compile.ts#L51 is the issue here.

Replaced it with a random string, and re-built everything and it seemed to work.

I'll look for a way to replace that generated with something that makes sense, and I'll update the Python examples. Thanks for the detailed bug report @gabe-l-hart !

gabe-l-hart commented 4 years ago

Thank you for the investigation! I'll keep an eye out here for an updated release with the new changes.