Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Improve global-sidecar request forwarding capabilities #47

Merged
merged 1 commit into from
Apr 24, 2022

Conversation

cywang1905
Copy link
Contributor

@cywang1905 cywang1905 commented Apr 19, 2022

Problem Solved

Global-sidecar would calculate the destination address to be forwarded based on the request header "Slime-Orig-Dest" when forwarding the request previously. When the request header "Slime-Orig-Dest" is empty, forwarding is rejected.

This PR extends the use of the global-sidecar scenario. When the request header "Slime-Orig-Dest" is empty, global-sidecar uses the request.host and the port on which the global-sidecar accepts the request as the destinatiuon address. Then request forwards directly, without generating an accesslog metric.

disaster-recovery-arc

* The picture is not accurate enough. Traffic forwarded by global-sidecar is routed through the sidecarProxy when it reaches the destination, unless traffic hijacking has been disabled.

This allows business traffic to be directed to the global-sidecar for correct forwarding when there is a problem with some business sidecarProxy. This implies some sense of disaster recovery capability.

Notes

  • general.wormholePort defines requests of which service ports should be forwarded, connected by ";"
  • replicas of global-sidecar can be defined in component.globalSidecar.replicas
  • resources of global-sidecar can be defined in component.globalSidecar.resources

Usage Example

---
apiVersion: config.netease.com/v1alpha1
kind: SlimeBoot
metadata:
  name: lazyload
  namespace: mesh-operator
spec:
  image:
    pullPolicy: Always
    repository: registry.cn-hangzhou.aliyuncs.com/slimeio/slime-lazyload
    tag: master-78f3d09_linux_amd64-dirty_e09cde4
  module:
    - name: lazyload-test
      kind: lazyload
      enable: true
      general:
        wormholePort: # replace to your application service ports, and extend the list in case of multi ports
          - "9080"
          - "9090"
      global:
        misc:
          globalSidecarMode: cluster
          metricSourceType: accesslog
  component:
    globalSidecar:
      enable: true
      sidecarInject:
        enable: true # should be true
        mode: pod
        labels:
          sidecar.istio.io/inject: "true"
      replicas: 2
      resources:
        requests:
          cpu: 200m
          memory: 200Mi
        limits:
          cpu: 400m
          memory: 400Mi
      image:
        repository: registry.cn-hangzhou.aliyuncs.com/slimeio/slime-global-sidecar
        tag: master-5d96f4d_linux_amd64
      probePort: 28888

@cywang1905 cywang1905 force-pushed the master branch 2 times, most recently from dc580b1 to 84a6744 Compare April 22, 2022 03:44
@cywang1905 cywang1905 changed the title New feature: disaster recovery based on global-sidecar Improve global-sidecar request forwarding capabilities Apr 22, 2022
@cywang1905 cywang1905 added the enhancement New feature or request label Apr 22, 2022
@cywang1905 cywang1905 force-pushed the master branch 2 times, most recently from 3690966 to 70a905e Compare April 22, 2022 06:25
@@ -95,19 +95,25 @@ spec:
serviceAccountName: global-sidecar
containers:
- name: global-sidecar
env:
- name: wormholePorts
value: {{ join ";" $f.wormholePort | quote }}
Copy link
Contributor

Choose a reason for hiding this comment

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

, joined items are more common

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change to comma

@@ -95,19 +95,25 @@ spec:
serviceAccountName: global-sidecar
containers:
- name: global-sidecar
env:
- name: wormholePorts
Copy link
Contributor

Choose a reason for hiding this comment

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

WORMHOLE_PORTS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


log "github.com/sirupsen/logrus"

"slime.io/slime/modules/lazyload/pkg/proxy"
)

const WormholePorts = "wormholePorts"
Copy link
Contributor

Choose a reason for hiding this comment

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

EnvWormholePorts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

log.Println("Starting health check on :8080")
if err := http.ListenAndServe(":8080", handler); err != nil {
log.Println("Starting health check on :18080")
if err := http.ListenAndServe(":18080", handler); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

no hardcode. pass via env/config and etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add env "PROBE_PORT", default 8080

log.Println("Starting proxy server on", *addr)
if err := http.ListenAndServe(*addr, handler); err != nil {
log.Fatal("ListenAndServe:", err)
p, err := strconv.Atoi(whPort)
Copy link
Contributor

Choose a reason for hiding this comment

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

prepare int ports in advance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -72,7 +53,33 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, req *http.Request) {
}
}
}
log.Infof("received request, reqHost: %s", reqHost)
log.Infof("proxy received request, reqHost: %s", reqHost)
Copy link
Contributor

Choose a reason for hiding this comment

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

Infof? debug or trace level

Copy link
Contributor Author

@cywang1905 cywang1905 Apr 24, 2022

Choose a reason for hiding this comment

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

change to debug level

}
handler := &proxy.Proxy{WormholePort: p}

addr := flag.String("addr"+strconv.Itoa(idx), "0.0.0.0:"+whPort, "The addr of the application.")
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove addr

origDestIp = reqHost
}
}
log.Infof("proxy forward request to: %s:%d", origDestIp, origDestPort)
Copy link
Contributor

Choose a reason for hiding this comment

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

log level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

debug level

flag.Parse()

if idx != len(whPorts)-1 {
go handler.StartServer(addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

only the last server failure will cause the app to exit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change, main func will exit when all web servers are closed

pkg/proxy/http.go Show resolved Hide resolved
@@ -95,19 +95,29 @@ spec:
serviceAccountName: global-sidecar
containers:
- name: global-sidecar
env:
- name: PROBE_PORT
value: {{ default 8080 $gs.probePort | quote }}
Copy link
Contributor

Choose a reason for hiding this comment

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

this default port value is very likely to conflict with business svc port. we should replace it with an uncommon one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change to 18181

livenessProbe:
failureThreshold: 3
httpGet:
path: /healthz/live
port: 8080
port: {{ default 8080 $gs.probePort }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use a var to hold this and avoid writing it everywhere?

@YonkaFang YonkaFang merged commit 5d96f4d into slime-io:master Apr 24, 2022
@cywang1905 cywang1905 added new feature and removed enhancement New feature or request labels Oct 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants