Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AWS JupyterNotebook Image #1155

Merged
merged 1 commit into from May 15, 2020

Conversation

PatrickXYS
Copy link
Member

Which issue is resolved by this Pull Request:
Resolves #1125

AWS users require to have a by-default ECR image when creating JupyterNotebook Server.

Description of your changes:
Add on overlay folder called aws, and create kustomization.yaml and configmap_patch.yaml files to patch the jupyter-web-app's configmap with AWS ECR Container image.

Checklist:

  • Unit tests have been rebuilt:
    1. cd manifests/tests
    2. make generate-changed-only
    3. make test

@kubeflow-bot
Copy link
Contributor

This change is Reviewable

@PatrickXYS
Copy link
Member Author

/cc @Jeffwan @krishnadurai @kimwnasptd Can you take a look?

@PatrickXYS
Copy link
Member Author

/test kubeflow-manifests-presubmit

@Jeffwan
Copy link
Member

Jeffwan commented May 6, 2020

seems there's some test errors

INFO     root:util.py:72 failed to apply:  (kubeflow.error): Code 500 with message: coordinator Apply failed for gcp:  (
kubeflow.error): Code 400 with message: gcp apply could not update deployment manager: could not update deployment manag
er entries; Creating kfctl-75da error(400): BAD REQUEST
=============================== warnings summary ===============================
/usr/local/lib/python3.8/dist-packages/_pytest/junitxml.py:417
 /usr/local/lib/python3.8/dist-packages/_pytest/junitxml.py:417: PytestDeprecationWarning: The 'junit_family' default va
lue will change to 'xunit2' in pytest 6.0.
 Add 'junit_family=xunit1' to your pytest.ini file to keep the current format in future versions of pytest and silence t
his warning.
   _issue_warning_captured(deprecated.JUNIT_XML_DEFAULT_FAMILY, config.hook, 2)
/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py:943
 /usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py:943: DeprecationWarning: Using or importing the ABCs
from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working
   collections.MutableMapping.register(ParseResults)
/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py:3226
 /usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py:3226: DeprecationWarning: Using or importing the ABCs
 from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working
   elif isinstance( exprs, collections.Iterable ):
kfctl_go_test.py::test_build_kfctl_go
 kfctl_go_test.py:10: PytestExperimentalApiWarning: record_xml_attribute is an experimental feature
   def test_build_kfctl_go(record_xml_attribute, app_name, app_path, project, use_basic_auth,
kfctl_go_test.py::test_build_kfctl_go
 /mnt/test-data-volume/kubeflow-manifests-presubmit-e2e-1155-c32b9e6-3552-f6a2/src/kubeflow/kfctl/py/kubeflow/kfctl/test
ing/util/kfctl_go_test_utils.py:76: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the defaul
t Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
   config_spec = yaml.load(f)
-- Docs: https://docs.pytest.org/en/latest/warnings.html
- generated xml file: /mnt/test-data-volume/kubeflow-manifests-presubmit-e2e-1155-c32b9e6-3552-f6a2/output/artifacts/jun
it_kubeflow-manifests-presubmit-e2e-1155-c32b9e6-3552-f6a2/junit_kfctl-build-testkfctl_gcp_iap.xml -
=========================== short test summary info ============================
FAILED kfctl_go_test.py::test_build_kfctl_go - subprocess.CalledProcessError:...
================== 1 failed, 5 warnings in 826.20s (0:13:46) ===================

Copy link
Member

@Jeffwan Jeffwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PatrickXYS See comments.

@PatrickXYS PatrickXYS changed the title Add support for AWS JupyterNotebook Image [AWS] Add support for AWS JupyterNotebook Image May 12, 2020
@PatrickXYS
Copy link
Member Author

PatrickXYS commented May 13, 2020

/assign @Jeffwan

I also update kfctl_aws.yaml to integrate with aws overlay, since it utilizes the master branch, we should be good to go.

Copy link
Member

@Jeffwan Jeffwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update source file

@Jeffwan Jeffwan changed the title [AWS] Add support for AWS JupyterNotebook Image Add support for AWS JupyterNotebook Image May 14, 2020
@Jeffwan
Copy link
Member

Jeffwan commented May 14, 2020

/lgtm
/approve

@PatrickXYS
Copy link
Member Author

Seems like we also need approval from jupyter/OWNERS.

/cc @kimwnasptd @lluunn Can you take a look?

@PatrickXYS
Copy link
Member Author

Can someone from Kubeflow/approver take a look?

/cc @terrytangyuan @richardsliu

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jeffwan, terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AWS specific Container Image into jupyter-web-app configMap
6 participants