tao12345666333 commented on code in PR #1306:
URL:
https://github.com/apache/apisix-ingress-controller/pull/1306#discussion_r1000165361
##########
pkg/providers/apisix/apisix_upstream.go:
##########
@@ -339,6 +404,114 @@ func (c *apisixUpstreamController) sync(ctx
context.Context, ev *types.Event) er
return err
}
+func (c *apisixUpstreamController) updateExternalNodes(ctx context.Context, au
*configv2.ApisixUpstream, old *configv2.ApisixUpstream, newUps
*apisixv1.Upstream) error {
+ clusterName := c.Config.APISIX.DefaultClusterName
+
+ // TODO: if old is not nil, diff the external nodes change first
+
+ upsName := apisixv1.ComposeExternalUpstreamName(au.Namespace, au.Name)
+
+ ups, err := c.APISIX.Cluster(clusterName).Upstream().Get(ctx, upsName)
+ if err != nil {
+ if err != apisixcache.ErrNotFound {
+ log.Errorf("failed to get upstream %s: %s", upsName,
err)
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ // Do nothing if not found
+ } else {
+ nodes, err :=
c.translator.TranslateApisixUpstreamExternalNodes(au)
+ if err != nil {
+ log.Errorf("failed to translate upstream external nodes
%s: %s", upsName, err)
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ if newUps != nil {
+ newUps.Metadata = ups.Metadata
+ ups = newUps
+ }
+
+ ups.Nodes = nodes
+ if _, err :=
c.APISIX.Cluster(clusterName).Upstream().Update(ctx, ups); err != nil {
+ log.Errorw("failed to update external nodes upstream",
+ zap.Error(err),
+ zap.Any("upstream", ups),
+ zap.Any("ApisixUpstream", au),
+ zap.String("cluster", clusterName),
+ )
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ }
+ return nil
+}
+
+func (c *apisixUpstreamController) syncRelationship(ev *types.Event, auKey
string, au kube.ApisixUpstream) {
+ obj := ev.Object.(kube.ApisixUpstreamEvent)
+
+ if obj.GroupVersion != config.ApisixV2 {
+ return
+ }
+
+ var (
+ old *configv2.ApisixUpstream
+ newObj *configv2.ApisixUpstream
+ )
+
+ if ev.Type == types.EventUpdate {
+ old = obj.OldObject.V2()
+ } else if ev.Type == types.EventDelete {
+ old = ev.Tombstone.(kube.ApisixUpstream).V2()
+ }
+
+ if ev.Type != types.EventDelete {
+ newObj = au.V2()
+ }
+
+ var (
+ //oldExternalDomains []string
+ //newExternalDomains []string
+ oldExternalServices []string
+ newExternalServices []string
+ )
+ if old != nil {
+ for _, node := range old.Spec.ExternalNodes {
+ if node.Type == configv2.ExternalTypeDomain {
+ //oldExternalDomains =
append(oldExternalDomains, node.Name)
Review Comment:
Do we not implement the handling of this function?
##########
pkg/providers/apisix/apisix_upstream.go:
##########
@@ -339,6 +404,114 @@ func (c *apisixUpstreamController) sync(ctx
context.Context, ev *types.Event) er
return err
}
+func (c *apisixUpstreamController) updateExternalNodes(ctx context.Context, au
*configv2.ApisixUpstream, old *configv2.ApisixUpstream, newUps
*apisixv1.Upstream) error {
+ clusterName := c.Config.APISIX.DefaultClusterName
+
+ // TODO: if old is not nil, diff the external nodes change first
+
+ upsName := apisixv1.ComposeExternalUpstreamName(au.Namespace, au.Name)
+
+ ups, err := c.APISIX.Cluster(clusterName).Upstream().Get(ctx, upsName)
+ if err != nil {
+ if err != apisixcache.ErrNotFound {
+ log.Errorf("failed to get upstream %s: %s", upsName,
err)
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ // Do nothing if not found
+ } else {
+ nodes, err :=
c.translator.TranslateApisixUpstreamExternalNodes(au)
+ if err != nil {
+ log.Errorf("failed to translate upstream external nodes
%s: %s", upsName, err)
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ if newUps != nil {
+ newUps.Metadata = ups.Metadata
+ ups = newUps
+ }
+
+ ups.Nodes = nodes
+ if _, err :=
c.APISIX.Cluster(clusterName).Upstream().Update(ctx, ups); err != nil {
+ log.Errorw("failed to update external nodes upstream",
+ zap.Error(err),
+ zap.Any("upstream", ups),
+ zap.Any("ApisixUpstream", au),
+ zap.String("cluster", clusterName),
+ )
+ c.RecordEvent(au, corev1.EventTypeWarning,
utils.ResourceSyncAborted, err)
+ c.recordStatus(au, utils.ResourceSyncAborted, err,
metav1.ConditionFalse, au.GetGeneration())
+ return err
+ }
+ }
+ return nil
+}
+
+func (c *apisixUpstreamController) syncRelationship(ev *types.Event, auKey
string, au kube.ApisixUpstream) {
+ obj := ev.Object.(kube.ApisixUpstreamEvent)
+
+ if obj.GroupVersion != config.ApisixV2 {
+ return
+ }
+
+ var (
+ old *configv2.ApisixUpstream
+ newObj *configv2.ApisixUpstream
+ )
+
+ if ev.Type == types.EventUpdate {
+ old = obj.OldObject.V2()
+ } else if ev.Type == types.EventDelete {
+ old = ev.Tombstone.(kube.ApisixUpstream).V2()
+ }
+
+ if ev.Type != types.EventDelete {
+ newObj = au.V2()
+ }
+
+ var (
+ //oldExternalDomains []string
+ //newExternalDomains []string
+ oldExternalServices []string
+ newExternalServices []string
+ )
+ if old != nil {
+ for _, node := range old.Spec.ExternalNodes {
+ if node.Type == configv2.ExternalTypeDomain {
+ //oldExternalDomains =
append(oldExternalDomains, node.Name)
Review Comment:
Don't we implement this feature?
##########
pkg/kube/apisix/apis/config/v2/types.go:
##########
@@ -483,6 +493,27 @@ type ApisixUpstreamConfig struct {
Subsets []ApisixUpstreamSubset `json:"subsets,omitempty"
yaml:"subsets,omitempty"`
}
+// ApisixUpstreamExternalType is the external service type
+type ApisixUpstreamExternalType string
+
+const (
+ // ExternalTypeDomain type is a domain
+ // +k8s:deepcopy-gen=false
+ ExternalTypeDomain ApisixUpstreamExternalType = "Domain"
+
+ // ExternalTypeService type is a K8s ExternalName service
+ // +k8s:deepcopy-gen=false
+ ExternalTypeService ApisixUpstreamExternalType = "service"
Review Comment:
```suggestion
ExternalTypeService ApisixUpstreamExternalType = "Service"
```
I think using `Service` is more appropriate
##########
pkg/providers/apisix/translation/apisix_upstream.go:
##########
@@ -47,3 +56,116 @@ func (t *translator) translateService(namespace, svcName,
subset, svcResolveGran
ups.ID = id.GenID(ups.Name)
return ups, nil
}
+
+// TODO: Scheme (http/https)
+func (t *translator) TranslateApisixUpstreamExternalNodes(au
*v2.ApisixUpstream) ([]apisixv1.UpstreamNode, error) {
+ var nodes []apisixv1.UpstreamNode
+ for i, node := range au.Spec.ExternalNodes {
+ if node.Type == v2.ExternalTypeDomain {
+ arr := strings.Split(node.Name, ":")
+
+ weight := translation.DefaultWeight
+ if node.Weight != nil {
+ weight = *node.Weight
+ }
+ n := apisixv1.UpstreamNode{
+ Host: arr[0],
+ Weight: weight,
+ }
+
+ if len(arr) == 1 {
+ if strings.HasPrefix(arr[0], "https://";) {
+ n.Port = 443
+ } else {
+ n.Port = 80
+ }
+ } else if len(arr) == 2 {
+ port, err := strconv.Atoi(arr[1])
+ if err != nil {
+ return nil, errors.Wrap(err,
fmt.Sprintf("failed to parse ApisixUpstream %s/%s port: at ExternalNodes[%v]:
%s", au.Namespace, au.Name, i, node.Name))
+ }
+
+ n.Port = port
+ }
+
+ nodes = append(nodes, n)
+ } else if node.Type == v2.ExternalTypeService {
+ svc, err :=
t.ServiceLister.Services(au.Namespace).Get(node.Name)
+ if err != nil {
+ // In theory, ApisixRoute now watches all
service add event, a not found error is already handled
+ if k8serrors.IsNotFound(err) {
+ // TODO: Should retry
+ return nil, err
+ }
+ return nil, err
+ }
+
+ if svc.Spec.Type != corev1.ServiceTypeExternalName {
+ return nil, fmt.Errorf("ApisixUpstream %s/%s
ExternalNodes[%v] must refers to a ExternalName service: %s", au.Namespace,
au.Name, i, node.Name)
+ }
+
+ weight := translation.DefaultWeight
+ if node.Weight != nil {
+ weight = *node.Weight
+ }
+ n := apisixv1.UpstreamNode{
+ Host: svc.Spec.ExternalName,
+ Weight: weight,
+ }
+
+ arr := strings.Split(svc.Spec.ExternalName, ":")
+ if len(arr) == 1 {
+ if strings.HasPrefix(arr[0], "https://";) {
+ n.Port = 443
+ } else {
+ n.Port = 80
+ }
+ } else if len(arr) == 2 {
+ port, err := strconv.Atoi(arr[1])
+ if err != nil {
+ return nil, errors.Wrap(err,
fmt.Sprintf("failed to parse ApisixUpstream %s/%s port: at ExternalNodes[%v]:
%s", au.Namespace, au.Name, i, node.Name))
+ }
+
+ n.Port = port
+ }
Review Comment:
ExternalName Service is a DNS CNAME record.
According to RFC1123, it does not include protocols such as HTTP/HTTPS and
ports
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
