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 ingress annotation: kubernetes.io/ingress.class #2999

Merged
merged 1 commit into from
Sep 1, 2023
Merged

add ingress annotation: kubernetes.io/ingress.class #2999

merged 1 commit into from
Sep 1, 2023

Conversation

chenshaojin
Copy link
Contributor

What changes were proposed in this pull request

Issue Number: close #2998

@chenshaojin
Copy link
Contributor Author

cc @Al-assad

@Al-assad
Copy link
Member

Hi @chenshaojin, there are some code style issues in this pull request

@chenshaojin
Copy link
Contributor Author

你好@chenshaojin,此拉取请求中存在一些代码风格问题

ok, I will format the code style.

"nginx.ingress.kubernetes.io/rewrite-target" -> "/$2",
"nginx.ingress.kubernetes.io/proxy-body-size" -> "1024m",
"nginx.ingress.kubernetes.io/configuration-snippet" -> ("rewrite ^(/" + clusterId + ")$ $1/ permanent;")
)
val ingressClass = InternalConfigHolder.get[String](K8sFlinkConfig.ingressClass)
Copy link
Member

Choose a reason for hiding this comment

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

It can be improved to make the logic clearer:

val ingressClass = InternalConfigHolder.get[String](K8sFlinkConfig.ingressClass)
if (ingressClass.nonEmpty) {
  annotations + ("kubernetes.io/ingress.class" -> ingressClass)
}
annotations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@wolfboys
Copy link
Member

@chenshaojin there are some code style issues in PR, you can run command to fix it:
mvn spotless:apply

@chenshaojin
Copy link
Contributor Author

@chenshaojin there are some code style issues in PR, you can run command to fix it: mvn spotless:apply

I see. thx

@chenshaojin
Copy link
Contributor Author

cc @wolfboys

Copy link
Member

@wolfboys wolfboys left a comment

Choose a reason for hiding this comment

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

LGTM

@wolfboys wolfboys merged commit a703f2c into apache:dev Sep 1, 2023
6 checks passed
saLeox pushed a commit to saLeox/incubator-streampark that referenced this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] add ingresss annotations: kubernetes.io/ingress.class
3 participants