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

test(ws): add e2e tests #30

Merged
merged 3 commits into from
Aug 6, 2024
Merged

Conversation

Adembc
Copy link

@Adembc Adembc commented Jul 25, 2024

This PR makes the following changes to the workspace controller:

  • Remove the +kubebuilder:validation:XValidation:rule="self == oldSelf" annotations for image config and pod config.
  • Comment out probes as they prevent pod creation when empty:
    probes:
      # startupProbe: {}
      # livenessProbe: {}
      # readinessProbe: {}
    
  • Fix typo in extraEnv NB_PREFIX value from "juptyerlab" to "jupyterlab".
  • Remove installation and uninstallation of Prometheus Operator and Cert Manager in e2e tests - before and after each test.
  • Create the default-editor service account and delete it after testing.
  • Create home PVC and data PVC, and delete them afterwards.
  • Apply some WorkspaceKind samples.
  • Apply some Workspace samples.
  • Check if the workspace pod is running.
  • Test if the pod is exposed correctly through the service and path generated from the NB_PREFIX environment variable.

@Adembc
Copy link
Author

Adembc commented Jul 25, 2024

Now the unit tests are failing because I removed the validation that prevented the WorkspaceKind fields .Spec.PodTemplate.Options.ImageConfig and .Spec.PodTemplate.Options.PodConfig from being mutable. These validations were causing errors in the workspace reconciliation loop. As discussed with @thesuperzapper, these validations should be removed and replaced by validation webhooks.
Maybe we should merge the validation webhook PR first to ensure the unit tests pass.

@thesuperzapper thesuperzapper added project/notebooks-v2 project - kubeflow notebooks v2 area/ci area - related to ci labels Aug 1, 2024
@thesuperzapper thesuperzapper added this to the v2.0.0-alpha.0 milestone Aug 1, 2024
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@google-oss-prow google-oss-prow bot added size/XL and removed size/L labels Aug 6, 2024
@thesuperzapper
Copy link
Member

@Adembc I have done the following changes in 61c85fa

  • Auto detect the current kind cluster name for the kind load docker-image ... command.

@thesuperzapper
Copy link
Member

@Adembc I have done the following changes in 16886ce

  • Removed the tests which were failing because +kubebuilder:validation:XValidation:rule="self == oldSelf" was removed
  • Made the default image ghcr.io/kubeflow/notebooks/workspace-controller:latest (as this will be the image name once we release)
  • Grouped the samples under ./workspaces/controller/config/samples better to separate the "common" ones like ServiceAccount/default-editor and PVC/workspace-home-pvc from our actual custom CRDs.
  • Introduced a system to get Warning type Events from the StatefulSet and use them to pouplate the Workspace status.stateMessage when the Pod is missing.
    • note, this required adding a new Kubebuilder index on the .involvedObject.uid field of Events
  • Introduced new Workspace status.stateMessage information for cases when the Pod is unschedulable (e.g. due to no node meeting the CPU/RAM request)
  • Refactored the ./workspaces/controller/test/e2e/e2e_test.go:
    • to not run tests in the default namespace
    • to check that the Workspace status.state is Running
    • to properly run a curl pod that checks the service is up
  • Removed the now unused methods from ./workspaces/controller/test/utils/utils.go:
    • InstallPrometheusOperator()
    • UninstallPrometheusOperator()
    • UninstallCertManager()
    • InstallCertManager()

@thesuperzapper
Copy link
Member

@Adembc thanks for your work on this! We can do follow ups in future PRs.

/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thesuperzapper

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
Labels
approved area/ci area - related to ci lgtm project/notebooks-v2 project - kubeflow notebooks v2 size/XL
Projects
Development

Successfully merging this pull request may close these issues.

2 participants