You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2022/06/29 18:43:49 UTC
[GitHub] [servicecomb-service-center] wangshiqi308 opened a new pull request, #1309: [SCB-2617] Service center and istio integration
wangshiqi308 opened a new pull request, #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309
Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `go build` `go test` `go fmt` `go vet` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
- [x] Never comment source code, delete it.
- [x] UT should has "context, subject, expected result" result as test case name, when you call t.Run().
---
A tool that could sync service center service to istio, which enable service center and istio integration.
Here is the design proposal:
https://cwiki.apache.org/confluence/display/SERVICECOMB/servicecomb-service-center+istio+module
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914811175
##########
istio/pkg/controllers/servicecenter/client/client.go:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package client
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Client struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
+ AppInstanceWatcherCache map[string]string // Maps appId to id of a instance watcher service. Need app-specific watchers to avoid cross-app errors.
Review Comment:
please use sync.Map
##########
istio/pkg/controllers/servicecenter/client/client.go:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package client
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Client struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
Review Comment:
from the response i can see, u should not name it as Client, and the comments is also misleading the concept.
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914800690
##########
istio/pkg/controllers/servicecenter/controller.go:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "reflect"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+
+ client "github.com/apache/servicecomb-service-center/istio/pkg/controllers/servicecenter/client"
+ "istio.io/pkg/log"
+)
+
+type Controller struct {
+ // servicecomb service center go-chassis API client
+ client *client.Client
+ // Channel used to send and receive servicecomb service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of retrieved servicecomb service center microservices, mapped to their service ids
+ serviceCache map[string]*event.MicroserviceEntry
+}
+
+func NewController(addr string, e chan []event.ChangeEvent) *Controller {
+ controller := &Controller{
+ client: client.New(addr),
+ event: e,
+ serviceCache: map[string]*event.MicroserviceEntry{},
+ }
+
+ return controller
+}
+
+// Run until a stop signal is received
+func (c *Controller) Run(stop <-chan struct{}) {
Review Comment:
if you just want to control this goroutine, please use context, and wait for stop signal, not a channel
refer to https://www.prakharsrivastav.com/posts/golang-context-and-cancellation/
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910560911
##########
istio/docs/user_guide.md:
##########
@@ -0,0 +1,64 @@
+# User Guide
Review Comment:
mv all docs to /docs and use sphinx doc framework
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914777875
##########
istio/examples/consumer-provider/Readme.md:
##########
@@ -0,0 +1,70 @@
+# Context
Review Comment:
this "docs/user-guides/integration-istio.md" is incomplete, as a user my task is not just running a system proccess. please merge this part to docs/user-guides/integration-istio.md
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914797845
##########
istio/pkg/controllers/servicecenter/controller.go:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "reflect"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+
+ client "github.com/apache/servicecomb-service-center/istio/pkg/controllers/servicecenter/client"
+ "istio.io/pkg/log"
+)
+
Review Comment:
comments to describe the responsibility
##########
istio/pkg/controllers/servicecenter/controller.go:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "reflect"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+
+ client "github.com/apache/servicecomb-service-center/istio/pkg/controllers/servicecenter/client"
+ "istio.io/pkg/log"
+)
+
+type Controller struct {
+ // servicecomb service center go-chassis API client
+ client *client.Client
+ // Channel used to send and receive servicecomb service center change events from the service center controller
+ event chan []event.ChangeEvent
Review Comment:
events
##########
istio/pkg/controllers/servicecenter/controller.go:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "reflect"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+
+ client "github.com/apache/servicecomb-service-center/istio/pkg/controllers/servicecenter/client"
+ "istio.io/pkg/log"
+)
+
+type Controller struct {
+ // servicecomb service center go-chassis API client
+ client *client.Client
+ // Channel used to send and receive servicecomb service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of retrieved servicecomb service center microservices, mapped to their service ids
+ serviceCache map[string]*event.MicroserviceEntry
+}
+
+func NewController(addr string, e chan []event.ChangeEvent) *Controller {
+ controller := &Controller{
+ client: client.New(addr),
+ event: e,
+ serviceCache: map[string]*event.MicroserviceEntry{},
+ }
+
+ return controller
+}
+
+// Run until a stop signal is received
+func (c *Controller) Run(stop <-chan struct{}) {
Review Comment:
if you just want to control this goroutine, please use context, and wait for cancel signal, not a channel
##########
istio/pkg/controllers/servicecenter/controller.go:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "reflect"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+
+ client "github.com/apache/servicecomb-service-center/istio/pkg/controllers/servicecenter/client"
+ "istio.io/pkg/log"
+)
+
+type Controller struct {
+ // servicecomb service center go-chassis API client
+ client *client.Client
+ // Channel used to send and receive servicecomb service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of retrieved servicecomb service center microservices, mapped to their service ids
+ serviceCache map[string]*event.MicroserviceEntry
Review Comment:
how about use sync.Map
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914779527
##########
istio/examples/consumer-provider/Readme.md:
##########
@@ -0,0 +1,70 @@
+# Context
+We have two services: Provider and Consumer:
+* `provider` is the provider of a service which calculates and returns the sum of the square root from 1 to a user provided parameter `x`.
+* `consumer` performs as both provider and consumer. As a consumer, it calls the api provided by `provider` to get the result of the sum of square roots;
+as a `provider`, it provides a service externally which returns the result it gets from `provider` to its clients.
+
+While Provider uses servicecomb service center tech stack, Consumer uses istio tech stack. Origionaly, Provider and Consumer couldn't discover each other.
+
+In this demo, we are going to adopt our servicecomb-service-center-istio to brake the barrier between Provider and Consumer.
+
+The logical network and calling relationship of the Provider and Consumer is as follows：
+
+
+
+# Pre-use preparation
+Please follow [user-guide](../../docs/user_guide.md) to install all the required environment as well as the servicecomb-service-center-istio tool.
+
+# Build Provider and Consumer service
+## Consumer
+```
+# go to consumer folder and run
+> go build -o consumer main.go
Review Comment:
please use docker multi-stage build to improve the ux
##########
istio/examples/consumer-provider/Readme.md:
##########
@@ -0,0 +1,70 @@
+# Context
Review Comment:
this "docs/user-guides/integration-istio.md" is imcomplete, as a user my task is not just running a system proccess. please merge this part to docs/user-guides/integration-istio.md
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] wangshiqi308 commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
wangshiqi308 commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r915952419
##########
istio/pkg/controllers/istioconnector/controller.go:
##########
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package istioconnector
+
+import (
+ "context"
+ "fmt"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+ "istio.io/client-go/pkg/apis/networking/v1alpha3"
+ "istio.io/client-go/pkg/clientset/versioned"
+ "istio.io/pkg/log"
+ v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/clientcmd"
+ k8s "sigs.k8s.io/controller-runtime/pkg/client/config"
+)
+
+// Controller receives service center updates and pushes converted Istio ServiceEntry(s) to k8s api server
+type Controller struct {
+ // Istio istioClient for k8s API
+ istioClient *versioned.Clientset
+ // Channel used to send and receive service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of converted service entries, mapped to original service center service id
+ convertedServiceCache map[string]*v1alpha3.ServiceEntry
+}
+
+func NewController(kubeconfigPath string, e chan []event.ChangeEvent) (*Controller, error) {
+ controller := &Controller{
+ event: e,
+ convertedServiceCache: make(map[string]*v1alpha3.ServiceEntry),
+ }
+
+ // get kubernetes config info, used for creating k8s client
+ client, err := newKubeClient(kubeconfigPath)
+ if err != nil {
+ log.Errorf("Failed to create istio client: %v\n", err)
+ return nil, err
+ }
+ controller.istioClient = client
+ return controller, nil
+}
+
+// Return a debounced version of a function `fn` that will not run until `wait` seconds have passed
+// after it was last called or until `maxWait` seconds have passed since its first call.
+// Once `fn` is executed, the max wait timer is reset.
+func debounce(fn func(), wait time.Duration, maxWait time.Duration) func() {
+ // Main timer, time seconds elapsed since last execution
+ var timer *time.Timer
+ // Max wait timer, time seconds elapsed since first call
+ var maxTimer *time.Timer
+ return func() {
+ if maxTimer == nil {
+ // First debounced event, start max wait timer
+ // will only run target func if not called again after `maxWait` duration
+ maxTimer = time.AfterFunc(maxWait, func() {
+ // Reset all timers when max wait time is reached
+ log.Debugf("Debounce: maximum wait time reached, running target fn\n")
+ if timer.Stop() {
+ // Only run target func if main timer hasn't already
+ fn()
+ }
+ timer = nil
+ maxTimer = nil
+ })
+ log.Debugf("Debounce: max timer started, will wait max time of %s\n", maxWait)
+ }
+ if timer != nil {
+ // Timer already started; function was called within `wait` duration, debounce this event by resetting timer
+ timer.Stop()
+ }
+ // Start timer, will only run target func if not called again after `wait` duration
+ timer = time.AfterFunc(wait, func() {
+ log.Debugf("Debounce: timer completed, running target fn\n")
+ // Reset all timers and run target func when wait time is reached
+ fn()
+ maxTimer.Stop()
+ maxTimer = nil
+ timer = nil
+ })
+ log.Debugf("Debounce: timer started, will wait %s\n", wait)
+ }
+}
+
+// Run until a signal is received, this function won't block
+func (c *Controller) Run(stop <-chan struct{}) {
+ go c.watchServiceCenterUpdate(stop)
+}
+
+func (c *Controller) Stop() {
+
Review Comment:
Removed this function, no longer needed
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] wangshiqi308 commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
wangshiqi308 commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r918236631
##########
docs/user-guides/integration-istio.md:
##########
@@ -0,0 +1,119 @@
+# User Guide
+This instructions will lead you to getting start with using Servicecomb-service-center-istio
+
+## 1. Install dependencies
+This tool can be used both inside a k8s cluster and a standalone service running on a VM.
+
+For both ways you have to install dependencies first.
+
+### 1.1 Install Kubernetes Cluster
+You can follow K8S [installation instruction](https://kubernetes.io/docs/setup/) to install a K8S cluster
+
+### 1.2 Install Istio
+Follow this [instruction](https://istio.io/latest/docs/setup/getting-started/) to install istio
+
+**note: the instruction is just a show case of how to install and use istio, if you want to use it in production, you have to use a production ready installation profile**
+
+### 1.3 Install Istio DNS
+As any Servicecomb service center service will be translated to Serviceentry in K8S, while Kubernetes provides DNS resolution for Kubernetes Services out of the box, any custom ServiceEntrys will not be recognized. In addition to capturing application traffic, Istio can also capture DNS requests to improve the performance and usability of your mesh
+
+Use the following command to install istio DNS:
+```
+cat <<EOF | istioctl install -y -f -
+apiVersion: install.istio.io/v1alpha1
+kind: IstioOperator
+spec:
+ meshConfig:
+ defaultConfig:
+ proxyMetadata:
+ # Enable basic DNS proxying
+ ISTIO_META_DNS_CAPTURE: "true"
+ # Enable automatic address allocation, optional
+ ISTIO_META_DNS_AUTO_ALLOCATE: "true"
+EOF
+```
+
+### 1.4 Install Servicecomb service center
+Servicecomb service center could be installed in K8S or on VM.
+Install Servicecomb service center follow this [instruction](https://github.com/apache/servicecomb-service-center/blob/master/README.md)
+
+## 2 Install Servicecomb-service-center-istio
+### 2.1 Building
+You don’t need to build from source to use Servicecomb-service-center-istio (binaries in apache nexus ), but if you want to try out the latest and greatest, Servicecomb-service-center-istio can be easily built.
+```
+go build -o servicecomb-service-center-istio cmd/main.go
+```
+
+### 2.2 Building docker image
+```
+docker build -t servicecomb-service-center-istio:dev .
+```
+
+### 2.2 Run on VM
+```
+./servicecomb-service-center-istio --sc-addr=?SERVICE_CENTER_ADDRESS --kube-config=?KUBE_CONFIG_FILE_PATH
+```
+
+### 2.3 Run in K8S
+```
+# make sure you modified the input args in the deployment.yaml file first, specify you service center address
+kubectl apply -f manifest/deployment.yaml
+```
+
+### 2.4 Input parameters
+
+
+## 3 Best Practices
+We will use [consumer-provider](../../istio/examples/consumer-provider/) example to show how to use this tool.
+
+We have two services: Provider and Consumer:
+* `provider` is the provider of a service which calculates and returns the sum of the square root from 1 to a user provided parameter `x`.
+* `consumer` performs as both provider and consumer. As a consumer, it calls the api provided by `provider` to get the result of the sum of square roots;
+as a `provider`, it provides a service externally which returns the result it gets from `provider` to its clients.
+
+While Provider uses servicecomb service center tech stack, Consumer uses istio tech stack. Origionaly, Provider and Consumer couldn't discover each other.
+
+In this demo, we are going to adopt our servicecomb-service-center-istio to brake the barrier between Provider and Consumer.
+
+The logical network and calling relationship of the Provider and Consumer is as follows：
+
+
+
+### 3.1 Build Provider and Consumer service images
Review Comment:
I removed the image here as I have already clearly described each component in this example
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r917724355
##########
docs/user-guides/integration-istio.md:
##########
@@ -0,0 +1,119 @@
+# User Guide
+This instructions will lead you to getting start with using Servicecomb-service-center-istio
+
+## 1. Install dependencies
+This tool can be used both inside a k8s cluster and a standalone service running on a VM.
+
+For both ways you have to install dependencies first.
+
+### 1.1 Install Kubernetes Cluster
+You can follow K8S [installation instruction](https://kubernetes.io/docs/setup/) to install a K8S cluster
+
+### 1.2 Install Istio
+Follow this [instruction](https://istio.io/latest/docs/setup/getting-started/) to install istio
+
+**note: the instruction is just a show case of how to install and use istio, if you want to use it in production, you have to use a production ready installation profile**
+
+### 1.3 Install Istio DNS
+As any Servicecomb service center service will be translated to Serviceentry in K8S, while Kubernetes provides DNS resolution for Kubernetes Services out of the box, any custom ServiceEntrys will not be recognized. In addition to capturing application traffic, Istio can also capture DNS requests to improve the performance and usability of your mesh
+
+Use the following command to install istio DNS:
+```
+cat <<EOF | istioctl install -y -f -
+apiVersion: install.istio.io/v1alpha1
+kind: IstioOperator
+spec:
+ meshConfig:
+ defaultConfig:
+ proxyMetadata:
+ # Enable basic DNS proxying
+ ISTIO_META_DNS_CAPTURE: "true"
+ # Enable automatic address allocation, optional
+ ISTIO_META_DNS_AUTO_ALLOCATE: "true"
+EOF
+```
+
+### 1.4 Install Servicecomb service center
+Servicecomb service center could be installed in K8S or on VM.
+Install Servicecomb service center follow this [instruction](https://github.com/apache/servicecomb-service-center/blob/master/README.md)
+
+## 2 Install Servicecomb-service-center-istio
+### 2.1 Building
+You don’t need to build from source to use Servicecomb-service-center-istio (binaries in apache nexus ), but if you want to try out the latest and greatest, Servicecomb-service-center-istio can be easily built.
+```
+go build -o servicecomb-service-center-istio cmd/main.go
+```
+
+### 2.2 Building docker image
+```
+docker build -t servicecomb-service-center-istio:dev .
+```
+
+### 2.2 Run on VM
+```
+./servicecomb-service-center-istio --sc-addr=?SERVICE_CENTER_ADDRESS --kube-config=?KUBE_CONFIG_FILE_PATH
+```
+
+### 2.3 Run in K8S
+```
+# make sure you modified the input args in the deployment.yaml file first, specify you service center address
+kubectl apply -f manifest/deployment.yaml
+```
+
+### 2.4 Input parameters
+
+
+## 3 Best Practices
+We will use [consumer-provider](../../istio/examples/consumer-provider/) example to show how to use this tool.
+
+We have two services: Provider and Consumer:
+* `provider` is the provider of a service which calculates and returns the sum of the square root from 1 to a user provided parameter `x`.
+* `consumer` performs as both provider and consumer. As a consumer, it calls the api provided by `provider` to get the result of the sum of square roots;
+as a `provider`, it provides a service externally which returns the result it gets from `provider` to its clients.
+
+While Provider uses servicecomb service center tech stack, Consumer uses istio tech stack. Origionaly, Provider and Consumer couldn't discover each other.
+
+In this demo, we are going to adopt our servicecomb-service-center-istio to brake the barrier between Provider and Consumer.
+
+The logical network and calling relationship of the Provider and Consumer is as follows：
+
+
+
+### 3.1 Build Provider and Consumer service images
Review Comment:
3.1 chapter should name as "Example"， and simply draw a toplogy of this example along with istio and service center
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910560409
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
Review Comment:
move istio/cmd to /cmd rename app to istio
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914787453
##########
istio/pkg/controllers/istioconnector/controller_test.go:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package istioconnector
+
+import (
+ "testing"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestDebouncing(t *testing.T) {
+ pushed := 0
Review Comment:
please use t.Run to seperate test case, and the name should be given......when....then....
consider about reading UT "FIRST" principle
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910571383
##########
istio/docs/user_guide.md:
##########
@@ -0,0 +1,64 @@
+# User Guide
+This instructions will lead you to getting start with using Servicecomb-service-center-istio
+
+## 1. Install dependencies
+This tool can be used both inside a k8s cluster and a standalone service running on a VM.
+
+For both ways you have to install dependencies first.
+
+### 1.1 Install Kubernetes Cluster
+You can follow K8S [installation instruction](https://kubernetes.io/docs/setup/) to install a K8S cluster
+
+### 1.2 Install Istio
+Follow this [instruction](https://istio.io/latest/docs/setup/getting-started/) to install istio
+
+**note: the instruction is just a show case of how to install and use istio, if you want to use it in production, you have to use a production ready installation profile**
+
+### 1.3 Install Istio DNS
+As any Servicecomb service center service will be translated to Serviceentry in K8S, while Kubernetes provides DNS resolution for Kubernetes Services out of the box, any custom ServiceEntrys will not be recognized. In addition to capturing application traffic, Istio can also capture DNS requests to improve the performance and usability of your mesh
+
+Use the following command to install istio DNS:
+```
+cat <<EOF | istioctl install -y -f -
+apiVersion: install.istio.io/v1alpha1
+kind: IstioOperator
+spec:
+ meshConfig:
+ defaultConfig:
+ proxyMetadata:
+ # Enable basic DNS proxying
+ ISTIO_META_DNS_CAPTURE: "true"
+ # Enable automatic address allocation, optional
+ ISTIO_META_DNS_AUTO_ALLOCATE: "true"
+EOF
+```
+
+### 1.4 Install Servicecomb service center
+Servicecomb service center could be installed in K8S or on VM.
+Install Servicecomb service center follow this [instruction](https://github.com/apache/servicecomb-service-center/blob/master/README.md)
+
+## 2 Install Servicecomb-service-center-istio
+### 2.1 Building
+You don’t need to build from source to use Servicecomb-service-center-istio (binaries in apache nexus ), but if you want to try out the latest and greatest, Servicecomb-service-center-istio can be easily built.
+```
+go build -o servicecomb-service-center-istio cmd/main.go
+```
+
+### 2.2 Building docker image
+```
+docker build -t servicecomb-service-center-istio:dev .
+```
+
+### 2.2 Run on VM
+```
+./Servicecomb-service-center-istio --sc-addr=?SERVICE_CENTER_ADDRESS --kube-config=?KUBE_CONFIG_FILE_PATH
Review Comment:
change Servicecomb to lower case
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910560911
##########
istio/docs/user_guide.md:
##########
@@ -0,0 +1,64 @@
+# User Guide
Review Comment:
mvn all docs to /docs and use sphinx doc framework
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914810018
##########
istio/pkg/controllers/servicecenter/client/client.go:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package client
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Client struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
+ AppInstanceWatcherCache map[string]string // Maps appId to id of a instance watcher service. Need app-specific watchers to avoid cross-app errors.
+}
+
+func New(addr string) *Client {
+ registryClient, err := sc.NewClient(
+ sc.Options{
+ Endpoints: []string{addr},
+ })
+ if err != nil {
+ log.Errorf("Failed to create service center client, err[%v]\n", err)
+ }
+ return &Client{
+ client: registryClient,
+ AppInstanceWatcherCache: map[string]string{},
+ }
+}
+
+// Check whether a service center MicroService exists in the registry.
+func (c *Client) GetServiceExistence(microServiceId string) bool {
+ s, _ := c.client.GetMicroService(microServiceId, sc.WithGlobal())
+ return s != nil
+}
+
+// Retrieve all service center MicroServices, without their instances, from the registry.
+func (c *Client) GetAllServices() ([]*discovery.MicroService, error) {
+ microservices, err := c.client.GetAllMicroServices(sc.WithGlobal())
+ if err != nil {
+ return nil, err
+ }
+ return microservices, err
+}
+
+// Register a new service center Watcher service that watches instance-level change events for all service center services sharing a specific appId.
+func (c *Client) RegisterAppInstanceWatcher(name string, appId string, callback func(event event.ChangeEvent)) (string, error) {
+ watcherService := &discovery.MicroService{
+ AppId: appId,
+ ServiceName: name,
+ Environment: "",
+ Version: "0.0.1",
+ }
+
+ prevId, _ := c.client.GetMicroServiceID(watcherService.AppId, watcherService.ServiceName, "0.0.1", watcherService.Environment, sc.WithGlobal())
Review Comment:
why not check error
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang merged pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang merged PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#issuecomment-1170691103
Please Use RTD doc to maintain documents
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] wangshiqi308 commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
wangshiqi308 commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r911599991
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
+
+import (
+ "fmt"
+ "os"
+ "os/signal"
+ "syscall"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/bootstrap"
+
+ "github.com/spf13/cobra"
+ "istio.io/pkg/log"
+)
+
+var inputArgs *bootstrap.Args
+var loggingOptions = log.DefaultOptions()
+
+// NewRootCommand creates servicecomb-service-center-istio service cli args
+func NewRootCommand() *cobra.Command {
+ rootCmd := &cobra.Command{
+ Use: "servicecomb-service-center-to-istio",
Review Comment:
I rename it to `servicecenter-to-istio`
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914783074
##########
istio/pkg/controllers/istioconnector/controller.go:
##########
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package istioconnector
+
+import (
+ "context"
+ "fmt"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+ "istio.io/client-go/pkg/apis/networking/v1alpha3"
+ "istio.io/client-go/pkg/clientset/versioned"
+ "istio.io/pkg/log"
+ v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/clientcmd"
+ k8s "sigs.k8s.io/controller-runtime/pkg/client/config"
+)
+
+// Controller receives service center updates and pushes converted Istio ServiceEntry(s) to k8s api server
+type Controller struct {
+ // Istio istioClient for k8s API
+ istioClient *versioned.Clientset
+ // Channel used to send and receive service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of converted service entries, mapped to original service center service id
+ convertedServiceCache map[string]*v1alpha3.ServiceEntry
+}
+
+func NewController(kubeconfigPath string, e chan []event.ChangeEvent) (*Controller, error) {
+ controller := &Controller{
+ event: e,
+ convertedServiceCache: make(map[string]*v1alpha3.ServiceEntry),
+ }
+
+ // get kubernetes config info, used for creating k8s client
+ client, err := newKubeClient(kubeconfigPath)
+ if err != nil {
+ log.Errorf("Failed to create istio client: %v\n", err)
+ return nil, err
+ }
+ controller.istioClient = client
+ return controller, nil
+}
+
+// Return a debounced version of a function `fn` that will not run until `wait` seconds have passed
+// after it was last called or until `maxWait` seconds have passed since its first call.
+// Once `fn` is executed, the max wait timer is reset.
+func debounce(fn func(), wait time.Duration, maxWait time.Duration) func() {
+ // Main timer, time seconds elapsed since last execution
+ var timer *time.Timer
+ // Max wait timer, time seconds elapsed since first call
+ var maxTimer *time.Timer
+ return func() {
+ if maxTimer == nil {
+ // First debounced event, start max wait timer
+ // will only run target func if not called again after `maxWait` duration
+ maxTimer = time.AfterFunc(maxWait, func() {
+ // Reset all timers when max wait time is reached
+ log.Debugf("Debounce: maximum wait time reached, running target fn\n")
+ if timer.Stop() {
+ // Only run target func if main timer hasn't already
+ fn()
+ }
+ timer = nil
+ maxTimer = nil
+ })
+ log.Debugf("Debounce: max timer started, will wait max time of %s\n", maxWait)
+ }
+ if timer != nil {
+ // Timer already started; function was called within `wait` duration, debounce this event by resetting timer
+ timer.Stop()
+ }
+ // Start timer, will only run target func if not called again after `wait` duration
+ timer = time.AfterFunc(wait, func() {
+ log.Debugf("Debounce: timer completed, running target fn\n")
+ // Reset all timers and run target func when wait time is reached
+ fn()
+ maxTimer.Stop()
+ maxTimer = nil
+ timer = nil
+ })
+ log.Debugf("Debounce: timer started, will wait %s\n", wait)
+ }
+}
+
+// Run until a signal is received, this function won't block
+func (c *Controller) Run(stop <-chan struct{}) {
+ go c.watchServiceCenterUpdate(stop)
+}
+
+func (c *Controller) Stop() {
+
+}
+
+// Return a debounced version of the push2istio method that merges the passed events on each call.
+func (c *Controller) getIstioPushDebouncer(wait time.Duration, maxWait time.Duration, maxEvents int) func([]event.ChangeEvent) {
+ var eventQueue []event.ChangeEvent // Queue of events merged from arguments of each call to debounced function
+ // Make a debounced version of push2istio, with provided wait and maxWait times
+ debouncedFn := debounce(func() {
+ log.Debugf("Debounce: push callback fired, pushing events to Istio: %v\n", eventQueue)
+ // Timeout reached, push events to istio and reset queue
+ c.push2Istio(eventQueue)
+ eventQueue = nil
+ }, wait, maxWait)
+ return func(newEvents []event.ChangeEvent) {
+ log.Debugf("Debounce: received and merged %d new events\n", len(newEvents))
+ // Merge new events with existing event queue for each received call
+ eventQueue = append(eventQueue, newEvents...)
+
+ log.Debugf("Debounce: new total number of events in queue is %d\n", len(eventQueue))
Review Comment:
starts with lower case please
##########
istio/pkg/controllers/istioconnector/controller.go:
##########
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package istioconnector
+
+import (
+ "context"
+ "fmt"
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/go-chassis/cari/discovery"
+ "istio.io/client-go/pkg/apis/networking/v1alpha3"
+ "istio.io/client-go/pkg/clientset/versioned"
+ "istio.io/pkg/log"
+ v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/clientcmd"
+ k8s "sigs.k8s.io/controller-runtime/pkg/client/config"
+)
+
+// Controller receives service center updates and pushes converted Istio ServiceEntry(s) to k8s api server
+type Controller struct {
+ // Istio istioClient for k8s API
+ istioClient *versioned.Clientset
+ // Channel used to send and receive service center change events from the service center controller
+ event chan []event.ChangeEvent
+ // Lock for service cache
+ cacheMutex sync.Mutex
+ // Cache of converted service entries, mapped to original service center service id
+ convertedServiceCache map[string]*v1alpha3.ServiceEntry
+}
+
+func NewController(kubeconfigPath string, e chan []event.ChangeEvent) (*Controller, error) {
+ controller := &Controller{
+ event: e,
+ convertedServiceCache: make(map[string]*v1alpha3.ServiceEntry),
+ }
+
+ // get kubernetes config info, used for creating k8s client
+ client, err := newKubeClient(kubeconfigPath)
+ if err != nil {
+ log.Errorf("Failed to create istio client: %v\n", err)
+ return nil, err
+ }
+ controller.istioClient = client
+ return controller, nil
+}
+
+// Return a debounced version of a function `fn` that will not run until `wait` seconds have passed
+// after it was last called or until `maxWait` seconds have passed since its first call.
+// Once `fn` is executed, the max wait timer is reset.
+func debounce(fn func(), wait time.Duration, maxWait time.Duration) func() {
+ // Main timer, time seconds elapsed since last execution
+ var timer *time.Timer
+ // Max wait timer, time seconds elapsed since first call
+ var maxTimer *time.Timer
+ return func() {
+ if maxTimer == nil {
+ // First debounced event, start max wait timer
+ // will only run target func if not called again after `maxWait` duration
+ maxTimer = time.AfterFunc(maxWait, func() {
+ // Reset all timers when max wait time is reached
+ log.Debugf("Debounce: maximum wait time reached, running target fn\n")
+ if timer.Stop() {
+ // Only run target func if main timer hasn't already
+ fn()
+ }
+ timer = nil
+ maxTimer = nil
+ })
+ log.Debugf("Debounce: max timer started, will wait max time of %s\n", maxWait)
+ }
+ if timer != nil {
+ // Timer already started; function was called within `wait` duration, debounce this event by resetting timer
+ timer.Stop()
+ }
+ // Start timer, will only run target func if not called again after `wait` duration
+ timer = time.AfterFunc(wait, func() {
+ log.Debugf("Debounce: timer completed, running target fn\n")
+ // Reset all timers and run target func when wait time is reached
+ fn()
+ maxTimer.Stop()
+ maxTimer = nil
+ timer = nil
+ })
+ log.Debugf("Debounce: timer started, will wait %s\n", wait)
+ }
+}
+
+// Run until a signal is received, this function won't block
+func (c *Controller) Run(stop <-chan struct{}) {
+ go c.watchServiceCenterUpdate(stop)
+}
+
+func (c *Controller) Stop() {
+
Review Comment:
why not implement
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914773176
##########
istio/pkg/controllers/servicecenter/client/client.go:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package client
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Client struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
+ AppInstanceWatcherCache map[string]string // Maps appId to id of a instance watcher service. Need app-specific watchers to avoid cross-app errors.
+}
+
+func New(addr string) *Client {
+ registryClient, err := sc.NewClient(
+ sc.Options{
+ Endpoints: []string{addr},
+ })
+ if err != nil {
+ log.Errorf("Failed to create service center client, err[%v]\n", err)
Review Comment:
use lower case at the beging of all log info
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914794614
##########
istio/pkg/event/service_center_entry_test.go:
##########
@@ -0,0 +1,225 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package event_test
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/go-chassis/cari/discovery"
+ istioAPI "istio.io/api/networking/v1alpha3"
+)
+
+func mockNewMicroserviceEntryMultipleInstance() event.MicroserviceEntry {
+ ms := &discovery.MicroService{
+ ServiceId: "testsvc123456",
+ AppId: "Test-App",
+ ServiceName: "Test-Svc",
+ }
+
+ var insts []*event.InstanceEntry
+ ie := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst1a",
+ Endpoints: []string{"rest://1.1.1.1:1111", "rest://1.1.1.1:2222?sslEnabled=true", "rest://1.1.1.1:3333"},
+ HostName: "test-svc-host-1",
+ },
+ }
+ ie1 := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst2b",
+ Endpoints: []string{"rest://2.2.2.2:1111", "rest://2.2.2.2:2222?sslEnabled=true", "rest://2.2.2.2:3333"},
+ HostName: "test-svc-host-2",
+ },
+ }
+ insts = append(insts, ie, ie1)
+
+ return event.MicroserviceEntry{
+ MicroService: ms,
+ Instances: insts,
+ }
+}
+
+func TestConvertMicroserviceToIstioMultipleInstance(t *testing.T) {
+ in := mockNewMicroserviceEntryMultipleInstance()
Review Comment:
not readable, please use t.Run and meaningful name
##########
istio/pkg/utils/config.go:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package utils
+
+import "time"
+
+// Params under this const is used for service center controller
+const (
+ // The name used for all created service center Watcher services.
+ WATCHER_SVC_NAME string = "SERVICECENTER2MESHWATCHER"
Review Comment:
SERVICECENTER2MESHWATCHER is hard for human to read
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914812161
##########
istio/pkg/controllers/servicecenter/client/client.go:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package client
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Client struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
Review Comment:
from the response i can see, u should not name it as Client, and the comments is also missleading the concept.
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914787453
##########
istio/pkg/controllers/istioconnector/controller_test.go:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package istioconnector
+
+import (
+ "testing"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/utils"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestDebouncing(t *testing.T) {
+ pushed := 0
Review Comment:
please use t.Run to seperate test case, and the name should be given......when....then....
consider about read UT "FIRST" principle
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r911565584
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
Review Comment:
the reason is？
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910562743
##########
istio/pkg/bootstrap/server.go:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package bootstrap
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/istio"
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/service_center"
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "k8s.io/client-go/kubernetes"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+
+ "istio.io/pkg/log"
+)
+
+// cli args
+type Args struct {
+ // servicecomb-service-center address
+ ServiceCenterAddr string
+ // kubeconfig file path
+ Kubeconfig string
+ // enable leader election or not for high abalibility
+ HA bool
+}
+
+const (
+ // leader election check locked resource namespace
+ lockNameSpace = "istio-system"
+ // leader election check locked resource name
+ resourceName = "servicecenter2mesh"
+ // LeaseDuration is the duration that non-leader candidates will
+ // wait to force acquire leadership. This is measured against time of
+ // last observed ack.
+ //
+ // A client needs to wait a full LeaseDuration without observing a change to
+ // the record before it can attempt to take over. When all clients are
+ // shutdown and a new set of clients are started with different names against
+ // the same leader record, they must wait the full LeaseDuration before
+ // attempting to acquire the lease. Thus LeaseDuration should be as short as
+ // possible (within your tolerance for clock skew rate) to avoid a possible
+ // long waits in the scenario.
+ //
+ // Core clients default this value to 15 seconds.
+ defaultLeaseDuration = 15 * time.Second
+ // RenewDeadline is the duration that the acting master will retry
+ // refreshing leadership before giving up.
+ //
+ // Core clients default this value to 10 seconds.
+ defaultRenewDeadline = 10 * time.Second
+ // RetryPeriod is the duration the LeaderElector clients should wait
+ // between tries of actions.
+ //
+ // Core clients default this value to 2 seconds.
+ defaultRetryPeriod = 2 * time.Second
+)
+
+type Server struct {
+ // service center controller watches service center update, and push to istio controller
+ serviceCenterController *service_center.Controller
+ // istio controller receives updates from service center controller and push to k8s api server
+ istioController *istio.Controller
Review Comment:
istio.Controller leads missunderstanding please use istioConnector.Controller
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
+
+import (
+ "fmt"
+ "os"
+ "os/signal"
+ "syscall"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/bootstrap"
+
+ "github.com/spf13/cobra"
+ "istio.io/pkg/log"
+)
+
+var inputArgs *bootstrap.Args
+var loggingOptions = log.DefaultOptions()
+
+// NewRootCommand creates servicecomb-service-center-istio service cli args
+func NewRootCommand() *cobra.Command {
+ rootCmd := &cobra.Command{
+ Use: "servicecomb-service-center-to-istio",
Review Comment:
servicecomb-to-istio
##########
istio/pkg/bootstrap/server.go:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package bootstrap
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/istio"
Review Comment:
istio/pkg/controllers/istio seems hard to stand your design, please use more specific pkg name
##########
istio/pkg/bootstrap/server.go:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package bootstrap
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/istio"
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/service_center"
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "k8s.io/client-go/kubernetes"
+ "k8s.io/client-go/rest"
+ "k8s.io/client-go/tools/leaderelection"
+ "k8s.io/client-go/tools/leaderelection/resourcelock"
+
+ "istio.io/pkg/log"
+)
+
+// cli args
+type Args struct {
+ // servicecomb-service-center address
+ ServiceCenterAddr string
+ // kubeconfig file path
+ Kubeconfig string
+ // enable leader election or not for high abalibility
+ HA bool
+}
+
+const (
+ // leader election check locked resource namespace
+ lockNameSpace = "istio-system"
+ // leader election check locked resource name
+ resourceName = "servicecenter2mesh"
+ // LeaseDuration is the duration that non-leader candidates will
+ // wait to force acquire leadership. This is measured against time of
+ // last observed ack.
+ //
+ // A client needs to wait a full LeaseDuration without observing a change to
+ // the record before it can attempt to take over. When all clients are
+ // shutdown and a new set of clients are started with different names against
+ // the same leader record, they must wait the full LeaseDuration before
+ // attempting to acquire the lease. Thus LeaseDuration should be as short as
+ // possible (within your tolerance for clock skew rate) to avoid a possible
+ // long waits in the scenario.
+ //
+ // Core clients default this value to 15 seconds.
+ defaultLeaseDuration = 15 * time.Second
+ // RenewDeadline is the duration that the acting master will retry
+ // refreshing leadership before giving up.
+ //
+ // Core clients default this value to 10 seconds.
+ defaultRenewDeadline = 10 * time.Second
+ // RetryPeriod is the duration the LeaderElector clients should wait
+ // between tries of actions.
+ //
+ // Core clients default this value to 2 seconds.
+ defaultRetryPeriod = 2 * time.Second
+)
+
+type Server struct {
+ // service center controller watches service center update, and push to istio controller
+ serviceCenterController *service_center.Controller
Review Comment:
service_center is bad naming convention
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r910563133
##########
istio/pkg/bootstrap/server.go:
##########
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package bootstrap
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/controllers/istio"
Review Comment:
istio/pkg/controllers/istio seems hard to understand your design, please use more specific pkg name
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r917726744
##########
istio/pkg/controllers/servicecenter/connector.go:
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package servicecenter
+
+import (
+ "sync"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+
+ "github.com/go-chassis/cari/discovery"
+ "github.com/go-chassis/sc-client"
+ "istio.io/pkg/log"
+)
+
+const (
+ // Time in seconds to wait before re-registering a deleted Servicecomb Service Center Watcher service
+ REREGISTER_INTERVAL time.Duration = time.Second * 5
+)
+
+// Servicecomb Service Center go-chassis client
+type Connector struct {
+ client *sc.Client
+ cacheMutex sync.Mutex
+ AppInstanceWatcherCache map[string]string // Maps appId to id of a instance watcher service. Need app-specific watchers to avoid cross-app errors.
Review Comment:
use syc.Map
##########
istio/pkg/event/event.go:
##########
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package event
+
+// Common interface for convertable service and instance structs used by Service center and Istio APIs.
+type Event interface {
Review Comment:
Event shouldn't has ability to convet himself to another Event.
plz use EntryTransformer.Transfer(e Entry) to decouple with different entries
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] wangshiqi308 commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
wangshiqi308 commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r911564928
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
Review Comment:
discussed with @little-cui , we agreed to put all the stuff but docs in the istio folder to decouple with service center.
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] wangshiqi308 commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
wangshiqi308 commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r911567233
##########
istio/cmd/app/cmd.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
Review Comment:
this is a tool for service center and istio integration, the user could choose to use it or not, and this tool runs in a standalone mode, therefore it is not necessary to put main function in the `/cmd` folder
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914794614
##########
istio/pkg/event/service_center_entry_test.go:
##########
@@ -0,0 +1,225 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package event_test
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/go-chassis/cari/discovery"
+ istioAPI "istio.io/api/networking/v1alpha3"
+)
+
+func mockNewMicroserviceEntryMultipleInstance() event.MicroserviceEntry {
+ ms := &discovery.MicroService{
+ ServiceId: "testsvc123456",
+ AppId: "Test-App",
+ ServiceName: "Test-Svc",
+ }
+
+ var insts []*event.InstanceEntry
+ ie := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst1a",
+ Endpoints: []string{"rest://1.1.1.1:1111", "rest://1.1.1.1:2222?sslEnabled=true", "rest://1.1.1.1:3333"},
+ HostName: "test-svc-host-1",
+ },
+ }
+ ie1 := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst2b",
+ Endpoints: []string{"rest://2.2.2.2:1111", "rest://2.2.2.2:2222?sslEnabled=true", "rest://2.2.2.2:3333"},
+ HostName: "test-svc-host-2",
+ },
+ }
+ insts = append(insts, ie, ie1)
+
+ return event.MicroserviceEntry{
+ MicroService: ms,
+ Instances: insts,
+ }
+}
+
+func TestConvertMicroserviceToIstioMultipleInstance(t *testing.T) {
+ in := mockNewMicroserviceEntryMultipleInstance()
Review Comment:
all of test case are not readable, please use t.Run with meaningful name
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914794614
##########
istio/pkg/event/service_center_entry_test.go:
##########
@@ -0,0 +1,225 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package event_test
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/apache/servicecomb-service-center/istio/pkg/event"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/go-chassis/cari/discovery"
+ istioAPI "istio.io/api/networking/v1alpha3"
+)
+
+func mockNewMicroserviceEntryMultipleInstance() event.MicroserviceEntry {
+ ms := &discovery.MicroService{
+ ServiceId: "testsvc123456",
+ AppId: "Test-App",
+ ServiceName: "Test-Svc",
+ }
+
+ var insts []*event.InstanceEntry
+ ie := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst1a",
+ Endpoints: []string{"rest://1.1.1.1:1111", "rest://1.1.1.1:2222?sslEnabled=true", "rest://1.1.1.1:3333"},
+ HostName: "test-svc-host-1",
+ },
+ }
+ ie1 := &event.InstanceEntry{
+ &discovery.MicroServiceInstance{
+ ServiceId: "testsvc123456",
+ InstanceId: "testinst2b",
+ Endpoints: []string{"rest://2.2.2.2:1111", "rest://2.2.2.2:2222?sslEnabled=true", "rest://2.2.2.2:3333"},
+ HostName: "test-svc-host-2",
+ },
+ }
+ insts = append(insts, ie, ie1)
+
+ return event.MicroserviceEntry{
+ MicroService: ms,
+ Instances: insts,
+ }
+}
+
+func TestConvertMicroserviceToIstioMultipleInstance(t *testing.T) {
+ in := mockNewMicroserviceEntryMultipleInstance()
Review Comment:
all of test case are not readable, please use t.Run and meaningful name
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a diff in pull request #1309: [SCB-2617] Service center and istio integration
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on code in PR #1309:
URL: https://github.com/apache/servicecomb-service-center/pull/1309#discussion_r914777875
##########
istio/examples/consumer-provider/Readme.md:
##########
@@ -0,0 +1,70 @@
+# Context
Review Comment:
this "docs/user-guides/integration-istio.md" is incomplete, as a user, my task is not just running some system proccesses. please merge this part to docs/user-guides/integration-istio.md
--
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: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org