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

[CI] Install kuberay operator in buildkite test #1308

Merged

Conversation

architkulkarni
Copy link
Contributor

Why are these changes needed?

This PR extends the basic kind-in-docker test to install the Kuberay operator.

Related issue number

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
@architkulkarni architkulkarni marked this pull request as ready for review August 9, 2023 21:01
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
- popd

- pushd helm-chart/kuberay-operator
- helm install kuberay-operator --set image.repository=kuberay/operator --set image.tag=nightly .
Copy link
Member

Choose a reason for hiding this comment

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

You can also consider using the --wait option with Helm CLI (https://helm.sh/docs/helm/helm_install/) because kubectl wait isn't a smart command. Relying solely on kubectl wait might introduce flakiness. A lot of flakiness in KubeRay CI was introduced by kubectl wait. You can refer #618 for some examples.

In this PR, what will happen if the CI pipeline executes kubectl wait after the helm install command but before the KubeRay operator Pod is created? I guess it will report error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, let me switch to helm --wait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

# Install KubeRay operator
- pushd ray-operator
- IMG=kuberay/operator:nightly make docker-build
Copy link
Member

Choose a reason for hiding this comment

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

Or make docker-image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I guess that will skip the unit tests and make the test run faster. Any other reason? Let me go ahead and make that change

Copy link
Member

Choose a reason for hiding this comment

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

I guess that will skip the unit tests and make the test run faster.

yes

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

LGTM

@architkulkarni architkulkarni merged commit 08b9908 into ray-project:master Aug 9, 2023
21 checks passed
blublinsky pushed a commit to blublinsky/kuberay that referenced this pull request Aug 15, 2023
This PR extends the basic kind-in-docker test to install the Kuberay operator.

---------

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
This PR extends the basic kind-in-docker test to install the Kuberay operator.

---------

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants