You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2021/06/04 15:47:18 UTC

[GitHub] [submarine] noidname01 opened a new pull request #598: SUBMARINE-835. Exposing method modified

noidname01 opened a new pull request #598:
URL: https://github.com/apache/submarine/pull/598


   ### What is this PR for?
   Modify the way to access Submarine in a cluster in documentation.
   
   ### What type of PR is it?
   [Improvement| Documentation]
   
   ### Todos
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-835
   
   ### How should this be tested?
   
   ### Screenshots (if appropriate)
   
   https://user-images.githubusercontent.com/55401762/120828366-07fdc980-c58f-11eb-8112-c9e83eee2857.mp4
   
   ### Questions:
   * Do the license files need updating? No
   * Are there breaking changes for older versions? No
   * Does this need new documentation? No
   


-- 
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] [submarine] noidname01 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r645931684



##########
File path: submarine-cloud-v2/controller.go
##########
@@ -1271,16 +1269,7 @@ func (c *Controller) syncHandler(workqueueItem WorkQueueItem) error {
 			return err
 		}
 
-		// Port-forwarding
-		// TODO:
-		//   (1) multi-tenant port-forwarding
-		//   (2) Basic operations: on/off/modify (change port)
-		//   (3) in-cluster
-		if action == ADD {
-			if !c.incluster {

Review comment:
       Oh I found that there's something I didn't notice,  I'll fix ASAP. 




-- 
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] [submarine] kevin85421 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
kevin85421 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r645923222



##########
File path: submarine-cloud-v2/controller.go
##########
@@ -1271,16 +1269,7 @@ func (c *Controller) syncHandler(workqueueItem WorkQueueItem) error {
 			return err
 		}
 
-		// Port-forwarding
-		// TODO:
-		//   (1) multi-tenant port-forwarding
-		//   (2) Basic operations: on/off/modify (change port)
-		//   (3) in-cluster
-		if action == ADD {
-			if !c.incluster {

Review comment:
       Do you have tested the deletion of submarine CR? I guess that the deletion will have some bugs.
   
   

##########
File path: submarine-cloud-v2/README.md
##########
@@ -57,21 +64,25 @@ go build -o submarine-operator
 kubectl create ns submarine-admin
 kubectl apply -n submarine-admin -f artifacts/examples/example-submarine.yaml
 
-# Step3: "submarine-operator" will perform port-forwarding automatically.
+# Step3: Exposing Service
+# Using minikube ip + NodePort
+$ minikube ip  # you'll get the IP address of minikube, ex: 192.168.49.2
 
-# Step4: View workbench (127.0.0.1:32080) with your web browser
+# Step4: View workbench
+# http://{minikube ip}:32080, ex: http://192.168.49.2:32080

Review comment:
       In my experience, `minikube ip` does not work in the development environment (MacOS), and thus it would be better to add method2 (port-forward command).
   




-- 
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] [submarine] kevin85421 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
kevin85421 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r646143397



##########
File path: submarine-cloud-v2/README.md
##########
@@ -57,21 +64,25 @@ go build -o submarine-operator
 kubectl create ns submarine-admin
 kubectl apply -n submarine-admin -f artifacts/examples/example-submarine.yaml
 
-# Step3: "submarine-operator" will perform port-forwarding automatically.
+# Step3: Exposing Service
+# Using minikube ip + NodePort
+$ minikube ip  # you'll get the IP address of minikube, ex: 192.168.49.2
 
-# Step4: View workbench (127.0.0.1:32080) with your web browser
+# Step4: View workbench
+# http://{minikube ip}:32080, ex: http://192.168.49.2:32080

Review comment:
       In the following figure, the branch is at commit "SUBMARINE-841. Modify hard-coded URLs in E2E test to soft-coded URLs".
   <img width="581" alt="ζˆͺεœ– 2021-06-06 δΈ‹εˆ10 44 34" src="https://user-images.githubusercontent.com/20109646/120928880-1d066400-c719-11eb-8bda-d2f29af4d2b4.png">
   




-- 
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] [submarine] noidname01 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r645931684



##########
File path: submarine-cloud-v2/controller.go
##########
@@ -1271,16 +1269,7 @@ func (c *Controller) syncHandler(workqueueItem WorkQueueItem) error {
 			return err
 		}
 
-		// Port-forwarding
-		// TODO:
-		//   (1) multi-tenant port-forwarding
-		//   (2) Basic operations: on/off/modify (change port)
-		//   (3) in-cluster
-		if action == ADD {
-			if !c.incluster {

Review comment:
       Oh I found that there's something I didn't notice,  I'll fix it ASAP. 




-- 
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] [submarine] noidname01 commented on pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 commented on pull request #598:
URL: https://github.com/apache/submarine/pull/598#issuecomment-854831201


   @kevin85421


-- 
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] [submarine] noidname01 closed pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 closed pull request #598:
URL: https://github.com/apache/submarine/pull/598


   


-- 
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] [submarine] noidname01 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r645931352



##########
File path: submarine-cloud-v2/README.md
##########
@@ -57,21 +64,25 @@ go build -o submarine-operator
 kubectl create ns submarine-admin
 kubectl apply -n submarine-admin -f artifacts/examples/example-submarine.yaml
 
-# Step3: "submarine-operator" will perform port-forwarding automatically.
+# Step3: Exposing Service
+# Using minikube ip + NodePort
+$ minikube ip  # you'll get the IP address of minikube, ex: 192.168.49.2
 
-# Step4: View workbench (127.0.0.1:32080) with your web browser
+# Step4: View workbench
+# http://{minikube ip}:32080, ex: http://192.168.49.2:32080

Review comment:
       Oh I will add it




-- 
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] [submarine] noidname01 commented on a change in pull request #598: SUBMARINE-835. Exposing method modified

Posted by GitBox <gi...@apache.org>.
noidname01 commented on a change in pull request #598:
URL: https://github.com/apache/submarine/pull/598#discussion_r646294904



##########
File path: submarine-cloud-v2/README.md
##########
@@ -57,21 +64,25 @@ go build -o submarine-operator
 kubectl create ns submarine-admin
 kubectl apply -n submarine-admin -f artifacts/examples/example-submarine.yaml
 
-# Step3: "submarine-operator" will perform port-forwarding automatically.
+# Step3: Exposing Service
+# Using minikube ip + NodePort
+$ minikube ip  # you'll get the IP address of minikube, ex: 192.168.49.2
 
-# Step4: View workbench (127.0.0.1:32080) with your web browser
+# Step4: View workbench
+# http://{minikube ip}:32080, ex: http://192.168.49.2:32080

Review comment:
       It seems that you miss the url to expose, but I don't know what's the reason.
   ![](https://user-images.githubusercontent.com/55401762/120966507-e6702e00-c798-11eb-9a7a-54b52d48041e.png)
   I will add method2 to make sure everyone can access to the service.




-- 
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