You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/31 10:00:47 UTC

[GitHub] [apisix-ingress-controller] lingsamuel opened a new pull request, #1306: feat: support external service

lingsamuel opened a new pull request, #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306

   Signed-off-by: Ling Samuel <li...@gmail.com><!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   - [x] New feature provided
   
   ### What this PR does / why we need it:
   Resolves #927 
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#issuecomment-1232748978

   This is a new implementation of #726 and #927  , PTAL.
   
   /cc @elvis-cai  @purekeeper @BrandonArp @NMichas 
   
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on a diff in pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#discussion_r1000221301


##########
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:
   This function is used to track references to external services so that we can be notified when the external service changes.
   The domain doesn't need such track.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#issuecomment-1242600637

   could you please resolve conflicts? thanks


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#issuecomment-1233751528

   All tests passed, just a linter 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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 merged pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
tao12345666333 merged PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#issuecomment-1232765561

   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1306](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (603c828) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/0b999ec1d3087f7bf0037acbe2d398eaa0efab2a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b999ec) will **increase** coverage by `0.43%`.
   > The diff coverage is `34.89%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1306      +/-   ##
   ==========================================
   + Coverage   39.80%   40.23%   +0.43%     
   ==========================================
     Files          75       75              
     Lines        6962     7118     +156     
   ==========================================
   + Hits         2771     2864      +93     
   - Misses       3892     3929      +37     
   - Partials      299      325      +26     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [pkg/providers/apisix/translation/apisix\_plugin.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X3BsdWdpbi5nbw==) | `57.54% <ø> (ø)` | |
   | [pkg/providers/apisix/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/providers/utils/string.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy91dGlscy9zdHJpbmcuZ28=) | `0.00% <0.00%> (ø)` | |
   | [...kg/providers/apisix/translation/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X3Vwc3RyZWFtLmdv) | `48.83% <35.29%> (-51.17%)` | :arrow_down: |
   | [pkg/providers/apisix/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `25.02% <35.83%> (+5.88%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#discussion_r991890144


##########
pkg/providers/apisix/apisix_upstream.go:
##########
@@ -242,6 +280,30 @@ func (c *apisixUpstreamController) sync(ctx context.Context, ev *types.Event) er
 	case config.ApisixV2:
 		au := multiVersioned.V2()
 
+		if len(au.Spec.ExternalNodes) != 0 {
+			var newUps *apisixv1.Upstream
+			if au.Spec != nil && ev.Type != types.EventDelete {

Review Comment:
   We should check au.Spec != nil first 



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on a diff in pull request #1306: feat: support external service

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1306:
URL: https://github.com/apache/apisix-ingress-controller/pull/1306#discussion_r1000222427


##########
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:
   APISIX requires a port and the port field has not been implemented yet. This is a temporary solution.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org