You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/03/23 07:55:43 UTC
[GitHub] [skywalking-kubernetes] chengshiwen opened a new pull request #67: Improve notes.txt
chengshiwen opened a new pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67
Improve notes.txt
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r600125880
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
I added a sample configuration for `nodePort` because it's used in the service `skywalking-kubernetes/chart/skywalking/templates/ui-svc.yaml`. Some users may choose `F5+NodePort` to deploy services instead of `Ingress`. If they want to configure the `nodePort`, but cannot find the configuration item, it will make him confused.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] wu-sheng commented on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819124998
Seems this PR doesn't have update anymore?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r603225764
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
30080 is just a sample. In some k8s, only operation people have the rights to to allocate node ports, and random node port is not allowed. It would be a better option to allow node port to be specified.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r600125880
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
I added a sample configuration for `nodePort` because it's used in the service `skywalking-kubernetes/chart/skywalking/templates/ui-svc.yaml`. Some users may choose `F5 + NodePort` to deploy services instead of `Ingress`. If they want to configure the `nodePort`, but cannot find the configuration item, it will make him confused. By the way, the `nodePort` can be configured to a specific value.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] innerpeacez commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
innerpeacez commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r603170178
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
30080 is meaningless, it is recommended to use the default value range (30000-32767) instead @chengshiwen
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r600125880
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
I added a sample configuration for `nodePort` because it's used in the service `skywalking-kubernetes/chart/skywalking/templates/ui-svc.yaml`. Some users may choose `F5+NodePort` to deploy services instead of Ingress
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] innerpeacez commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
innerpeacez commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r607442094
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
My mean is that the example does not need a specific port, it can be a default port range, or prompt the user in the readme file @chengshiwen
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] innerpeacez merged pull request #67: Improve notes.txt and nodePort configuration
Posted by GitBox <gi...@apache.org>.
innerpeacez merged pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819270135
@wu-sheng @innerpeacez @kezhenxu94 Some people may need the default random port, and some people may need a specific port. For a specific port, a configuration entry and sample of `nodePort` may be required. Could I add comments as follows?
```
## nodePort is the port on each node on which this service is exposed when type=NodePort or LoadBalancer
## Default: auto-allocated port if not specified. 30080 is just an example
##
# nodePort: 30080
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen edited a comment on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen edited a comment on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819270135
@wu-sheng @innerpeacez @kezhenxu94 Some people may need the default random port, and some people may need a specific port. For a specific port, a configuration entry and sample of `nodePort` may be required. Could I add comments as follows?
```
## nodePort is the port on each node on which this service is exposed when type=NodePort or LoadBalancer
## Default: auto-allocated port if not specified. 30080 is just an example
##
# nodePort: 30080
```
Related code line: https://github.com/apache/skywalking-kubernetes/blob/72469176d63b7552c14bb7f4f2652ca0f4bd5130/chart/skywalking/templates/ui-svc.yaml#L47
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r603225764
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
30080 is just an example. In some k8s, only operation people have the rights to to allocate node ports, and random node port is not allowed. It would be a better option to allow node port to be specified. A user can change it to any pre-allocated port by operation people.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] kezhenxu94 commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r599538785
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
I think we recommend the users to configure a ingress gateway instead of expose a node port, no? @hanahmily
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r600125880
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
I added a sample configuration for `nodePort` because it's used in the service `skywalking-kubernetes/chart/skywalking/templates/ui-svc.yaml`. Some users may choose `F5 + NodePort` to deploy services instead of `Ingress`. If they want to configure the `nodePort`, but cannot find the configuration item, it will make him confused.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r603225764
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
30080 is just a sample. In some k8s, only operation people have the rights to to allocate node ports, and random node port is not allowed. It would be a better option to allow node port to be specified. A user can change it to any pre-allocated port by operation people.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen edited a comment on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen edited a comment on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819270135
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen edited a comment on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen edited a comment on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819270135
@wu-sheng @innerpeacez @kezhenxu94 Some people may need the default random port, and some people may need a specific port. For a specific port, both configuration entry and sample of `nodePort` may be required. Could I add comments as follows?
```
## nodePort is the port on each node on which this service is exposed when type=NodePort
## Default: auto-allocated port if not specified. 30080 is just an example
##
# nodePort: 30080
```
Related code lines:
https://github.com/apache/skywalking-kubernetes/blob/72469176d63b7552c14bb7f4f2652ca0f4bd5130/chart/skywalking/templates/ui-svc.yaml#L47-L49
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen edited a comment on pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen edited a comment on pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#issuecomment-819270135
@wu-sheng @innerpeacez @kezhenxu94 Some people may need the default random port, and some people may need a specific port. For a specific port, a configuration entry and sample of `nodePort` may be required. Could I add comments as follows?
```
## nodePort is the port on each node on which this service is exposed when type=NodePort
## Default: auto-allocated port if not specified. 30080 is just an example
##
# nodePort: 30080
```
Related code line: https://github.com/apache/skywalking-kubernetes/blob/72469176d63b7552c14bb7f4f2652ca0f4bd5130/chart/skywalking/templates/ui-svc.yaml#L48
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] chengshiwen commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
chengshiwen commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r603225764
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ # nodePort: 30080
Review comment:
@innerpeacez 30080 is just an example. In some k8s, only operation people have the rights to to allocate node ports, and random node port is not allowed. It would be a better option to allow node port to be specified. A user can change it to any pre-allocated port by operation people.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-kubernetes] hanahmily commented on a change in pull request #67: Improve notes.txt
Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #67:
URL: https://github.com/apache/skywalking-kubernetes/pull/67#discussion_r599650725
##########
File path: chart/skywalking/values.yaml
##########
@@ -97,6 +97,7 @@ ui:
# clusterIP: None
externalPort: 80
internalPort: 8080
+ nodePort: 30080
Review comment:
nodePort seems a default value to the port number once the service type is `NodePort`, but`service.type` determines the service type, defaulting to `ClusterIP`.
and I prefer to use ingress to expose UI service, let's set `ingress.enabled` to `true`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org