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

WIP - Convert e2e test to Python test #442

Conversation

ChristianZaccaria
Copy link
Collaborator

Issue link

#441

What changes have been made

Main changes:

  1. kind.sh
    Currently the main changes are made to the kind.sh script for setting up our KinD environment and creating a user with limited permissions. Reference: https://medium.com/@lionelvillard/creating-users-in-kind-cluster-6c5ee35db3fe

The script creates a KinD cluster, then it gets the Kubernetes root certificate and private key to generate the user certificate, user key, and CSR. Once generated, it replaces the values for the server, certificate, and key in the kube config file for the new user to get access to the KinD cluster.

RBAC rules are then created for the new user, and a cleanup is performed on the copied and generated csr/crt/keys files.

Lastly, as part of this setup script, the codeflare-sdk is installed.

  1. mnist_raycluster_sdk_test.go
    In this PR the test has been refactored to simplify the process and to run the test as a real-user would do. Based on previous discussions, the job spec no longer needs to be specified, and perhaps there won’t be a need to create the configmap either. In fact, we will want to convert the test to a Python test and leverage the SDK directly.

  2. Moved the src codeflare_sdk files to the root directory as required by poetry to install the SDK.

Immediate TODOs:

  • Convert test to Python Test.
  • Take the first KinD node and redirect pod hostname requests there. This was previously done within the Job Spec.
  • Assert the job has been submitted (running).
  • Assert the job has completed successfully.

Verification steps

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

@ChristianZaccaria ChristianZaccaria added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 11, 2024
Copy link
Contributor

openshift-ci bot commented Jan 11, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from bobbins228. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@ChristianZaccaria ChristianZaccaria force-pushed the e2e-user-priviliges branch 5 times, most recently from 6d43dc4 to 5114f3e Compare January 11, 2024 19:27
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2024
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Bobbins228 Bobbins228 changed the title WIP - Convert e2e test to Python test + Unpriviliged user test case WIP - Convert e2e test to Python test Jan 23, 2024
@ChristianZaccaria
Copy link
Collaborator Author

Closing, however its contents can be relevant as a follow-on to this PR: #451

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants