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 required fields to job cluster samples #301

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

JaeguKim
Copy link
Contributor

@JaeguKim JaeguKim commented Mar 1, 2022

I found flinkoperator_v1beta1_flinkjobcluster.yaml and flinkoperator_v1beta1_remotejobjar.yaml missed required fields for v1beta1 flinkclusters.flinkoperator.k8s.io.

@@ -21,6 +21,7 @@ spec:
image:
name: flink:1.14.2
jobManager:
accessScope: Cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @JaeguKim Thanks for this! Which version are you using? I believe this was addressed and shouldn't be required now with the latest beta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using flink-cluster-crd.yaml which is latest beta I believe.
It looks like the accessScope is still required field of jobManager property and restartPolicy is also still required field of job property.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see what's going on, helm CRD is outdated.

Copy link
Contributor

@regadas regadas left a comment

Choose a reason for hiding this comment

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

Gonna go ahead and merge this. the accessScope with the latest CRD in #323 should not be required but doesn't heart having it here.

@regadas regadas merged commit e6ff988 into spotify:master Mar 15, 2022
@regadas
Copy link
Contributor

regadas commented Mar 15, 2022

Thanks for this @JaeguKim sorry the delay.

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.

2 participants