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

Make replicas optional for the worker spec #1154

Closed

Conversation

manifest
Copy link

@manifest manifest commented Jun 9, 2023

Why are these changes needed?

The replicas field must not be set when autoscaler is used because:

  • Autoscaler updates the field.
  • When GitOps systems such as FluxCD or ArgoCD are used, they keep reverting changes made by the autoscaler on every reconciliation.

Related issue number

Closes #1120

Checks

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

@kevin85421 kevin85421 self-requested a review June 9, 2023 17:11
@kevin85421 kevin85421 self-assigned this Jun 9, 2023
@kevin85421
Copy link
Member

I will review it today or tomorrow. Thanks!

@kevin85421
Copy link
Member

Related issue: #265

@kevin85421
Copy link
Member

Hi @manifest, could you follow this doc to fix the CI failure "operator-consistency-check / ray-operator-verify-crd-rbac"? Thanks!

@manifest manifest force-pushed the optional-worker-replicas branch 2 times, most recently from ba0fd0c to d7158b9 Compare June 16, 2023 13:19
@manifest
Copy link
Author

@kevin85421 I've got an error following the steps in the docs. How can I fix that?

# Python 3.10.12
pip install -r tests/config/requirements.txt
cd ray-operator

make build
go: creating new go.mod: module tmp
Downloading sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.0
...
/Users/manifest/development/github/kuberay/ray-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
bash: /Users/manifest/development/github/kuberay/ray-operator/bin/controller-gen: No such file or directory
make: *** [generate] Error 127
make manifests
...
/Users/manifest/development/github/kuberay/ray-operator/bin/controller-gen "crd:maxDescLen=100,trivialVersions=true,preserveUnknownFields=false,generateEmbeddedObjectMeta=true,allowDangerousTypes=true" rbac:roleName=kuberay-operator webhook paths="./..." output:crd:artifacts:config=config/crd/bases
bash: /Users/manifest/development/github/kuberay/ray-operator/bin/controller-gen: No such file or directory
make: *** [manifests] Error 127

@Yicheng-Lu-llll
Copy link
Contributor

Hi @manifest, You may need to use go1.17.6. See https://github.com/ray-project/kuberay/blob/master/ray-operator/DEVELOPMENT.md#use-go-v117.

The replicas field must not be set when autoscaler is used because:
- Autoscaler updates the field.
- When GitOps systems such as FluxCD or ArgoCD are used,
  they keep reverting changes made by the autoscaler on every reconciliation.
@manifest
Copy link
Author

I was now able to run consistency check steps. Thank you.

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.

Could you please test the autoscaling YAML files? It may throw an error for a nil pointer dereference. You should revisit all references to Replicas.

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.

[Feature] Make Replicas optional
3 participants