You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "Jcodelove (via GitHub)" <gi...@apache.org> on 2023/05/12 10:18:00 UTC
[GitHub] [incubator-streampark] Jcodelove opened a new pull request, #2741: [FIX]fix parse k8s version
Jcodelove opened a new pull request, #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741
<!--
Thank you for contributing to StreamPark! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
## Contribution Checklist
- If this is your first time, please read our contributor guidelines: [Submit Code](https://streampark.apache.org/community/submit_guide/submit_code).
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-streampark/issues).
- Name the pull request in the form "[Feature] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- If the PR is unfinished, add `[WIP]` in your PR title, e.g., `[WIP][Feature] Title of the pull request`.
-->
## What changes were proposed in this pull request
Issue Number: close #2732
<!--(For example: This pull request proposed to add checkstyle plugin).-->
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
fix parse k8s version
- when I use ACK and EKS, the Minor of k8s obtained from the client is not just numbers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove closed pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove closed pull request #2741: [FIX]fix parse k8s version
URL: https://github.com/apache/incubator-streampark/pull/2741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] senlizishi commented on pull request #2741: [FIX]fix parse k8s version
Posted by "senlizishi (via GitHub)" <gi...@apache.org>.
senlizishi commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545528984
LGTM @Al-assad cc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545604842
hi @Jcodelove Thanks for your contribution, Can you provide the testcase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#discussion_r1192517583
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressController.scala:
##########
@@ -30,8 +30,7 @@ object IngressController extends Logger {
private lazy val ingressStrategy: IngressStrategy = {
using(new DefaultKubernetesClient()) {
client =>
- val versionInfo = client.getVersion
- val version = s"${versionInfo.getMajor}.${versionInfo.getMinor}".toDouble
+ val version = ingressStrategy.parseSemantic(client.getVersion.getGitVersion)
Review Comment:
val version = VERSION_REGEXP.findFirstIn(client.getVersion.getGitVersion).get.toDouble
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] senlizishi commented on pull request #2741: [FIX]fix parse k8s version
Posted by "senlizishi (via GitHub)" <gi...@apache.org>.
senlizishi commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1544991527
Hi, @Jcodelove Thank you for your contribution. I have a question here. May I ask if there is any relevant information to prove whether the occurrence of `majorVersion=24+` is due to Alibaba Cloud K8S customization or whether the K8S standard version itself is `24+`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on a diff in pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on code in PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#discussion_r1192692353
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressController.scala:
##########
@@ -30,8 +30,7 @@ object IngressController extends Logger {
private lazy val ingressStrategy: IngressStrategy = {
Review Comment:
>
completed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545029950
> Hi, @Jcodelove Thank you for your contribution. I have a question here. May I ask if there is any relevant information to prove whether the occurrence of `majorVersion=24+` is due to Alibaba Cloud K8S customization or whether the K8S standard version itself is `24+`
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/component-base/version/base.go
According to the above code description, K8S standard version itself is numeric possibly followed by "+"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] senlizishi commented on pull request #2741: [FIX]fix parse k8s version
Posted by "senlizishi (via GitHub)" <gi...@apache.org>.
senlizishi commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545042983
Based on the information you provided, I found that it mentioned '**TODO: Deprecate gitMajor and gitMinor, use only gitVersion**'. I think it may be better for us to use **gitversion** to obtain the version in order to support later versions. What do you think? @Jcodelove
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#discussion_r1192514194
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressStrategy.scala:
##########
@@ -26,13 +26,27 @@ import java.io.File
trait IngressStrategy {
+ // splits a version string into numeric and "extra" parts
+ private val VERSION_MATCH_RE = "^\\s*v?([0-9]+(?:\\.[0-9]+)*)(.*)*$".r
Review Comment:
`private[this] val GIT_VERSION_REGEXP = "^v(\\d+\\.\\d+)".r`
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressController.scala:
##########
@@ -30,8 +30,7 @@ object IngressController extends Logger {
private lazy val ingressStrategy: IngressStrategy = {
using(new DefaultKubernetesClient()) {
client =>
- val versionInfo = client.getVersion
- val version = s"${versionInfo.getMajor}.${versionInfo.getMinor}".toDouble
+ val version = ingressStrategy.parseSemantic(client.getVersion.getGitVersion)
Review Comment:
val version = GIT_VERSION_REGEXP.findFirstIn(client.getVersion.getGitVersion).get.toDouble
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressStrategy.scala:
##########
@@ -26,13 +26,27 @@ import java.io.File
trait IngressStrategy {
+ // splits a version string into numeric and "extra" parts
+ private val VERSION_MATCH_RE = "^\\s*v?([0-9]+(?:\\.[0-9]+)*)(.*)*$".r
+
def ingressUrlAddress(
nameSpace: String,
clusterId: String,
clusterClient: ClusterClient[_]): String
def configureIngress(domainName: String, clusterId: String, nameSpace: String): Unit
+ /**
+ * The version information of kubernetes is based on the gitVersion field ParseSemantic parses a
+ * version string that exactly obeys the syntax and semantics of the "Semantic Versioning"
+ * specification (http://semver.org/) (although it ignores leading and trailing whitespace, and
+ * allows the version to be preceded by "v").
+ */
+ def parseSemantic(getGitVersion: String): Double = {
+ val numbers = VERSION_MATCH_RE.findFirstMatchIn(getGitVersion).get.group(1).split('.')
+ s"${numbers(0)}.${numbers(1)}".toDouble
+ }
Review Comment:
We can remove this method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Al-assad commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Al-assad (via GitHub)" <gi...@apache.org>.
Al-assad commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545542138
@Jcodelove Hi Jcodelove, please check the CI errors in this section:
https://github.com/apache/incubator-streampark/actions/runs/4957761895/jobs/8870077942?pr=2741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#discussion_r1192514194
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressStrategy.scala:
##########
@@ -26,13 +26,27 @@ import java.io.File
trait IngressStrategy {
+ // splits a version string into numeric and "extra" parts
+ private val VERSION_MATCH_RE = "^\\s*v?([0-9]+(?:\\.[0-9]+)*)(.*)*$".r
Review Comment:
We can remove `VERSION_MATCH_RE`
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressController.scala:
##########
@@ -30,8 +30,7 @@ object IngressController extends Logger {
private lazy val ingressStrategy: IngressStrategy = {
Review Comment:
defined before line 30:
`private[this] val VERSION_REGEXP = "^v(\\d+\\.\\d+)".r`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545597759
>
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#discussion_r1192521176
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/ingress/IngressController.scala:
##########
@@ -30,8 +30,7 @@ object IngressController extends Logger {
private lazy val ingressStrategy: IngressStrategy = {
Review Comment:
defined before line 30:
`private[this] val VERSION_REGEXP = "(\\d+\\.\\d+)".r`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545522535
> Based on the information you provided, I found that it mentioned '**TODO: Deprecate gitMajor and gitMinor, use only gitVersion**'. I think it may be better for us to use **gitversion** to obtain the version in order to support later versions. What do you think? @Jcodelove
I am revising this question, you can see what else is wrong
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545516731
fix commit log
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] Jcodelove commented on pull request #2741: [FIX]fix parse k8s version
Posted by "Jcodelove (via GitHub)" <gi...@apache.org>.
Jcodelove commented on PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741#issuecomment-1545598447
> @Jcodelove Hi Jcodelove, please check the CI errors in this section: https://github.com/apache/incubator-streampark/actions/runs/4957761895/jobs/8870077942?pr=2741
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys merged pull request #2741: [FIX]fix parse k8s version
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys merged PR #2741:
URL: https://github.com/apache/incubator-streampark/pull/2741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org