Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


AlinsRan merged PR #2460:
URL: https://github.com/apache/apisix-ingress-controller/pull/2460


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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


AlinsRan commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188930503


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }

Review Comment:
   done.



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


AlinsRan commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188930285


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+   log.Warnw("sync failed", zap.String("configName", configName), 
zap.Any("statuses", execErrors))
+   for _, execErr := range execErrors.Errors {
+   for _, failedStatus := range execErr.FailedErrors {
+   if len(failedStatus.FailedStatuses) == 0 {
+   resource, err := 
d.store.GetResources(execErr.Name)
+   if err != nil {
+   log.Errorw("failed to get 
resources from store", zap.String("configName", configName), zap.Error(err))
+   continue
+   }
+
+   fillStatusUpdateMapFunc := func(obj 
adctypes.Object) {
+   labels := obj.GetLabels()
+   statusKey := 
types.NamespacedNameKind{
+   Name:  
labels[label.LabelName],
+   Namespace: 
labels[label.LabelNamespace],
+   Kind:  
labels[label.LabelKind],
+   }
+   if msgs, ok := 
statusUpdateMap[statusKey]; ok {
+   
statusUpdateMap[statusKey] = append(msgs, failedStatus.Error())
+   } else {
+   
statusUpdateMap[statusKey] = []string{failedStatus.Error()}
+   }

Review Comment:
   fixed.



##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+ 

Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


AlinsRan commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188929996


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+   log.Warnw("sync failed", zap.String("configName", configName), 
zap.Any("statuses", execErrors))
+   for _, execErr := range execErrors.Errors {
+   for _, failedStatus := range execErr.FailedErrors {

Review Comment:
   Refactored.



##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {

Review Comment:
   fixed.



##
internal/provider/adc/store.go:
##
@@ -244,3 +245,79 @@ func (s *Store) GetResources(name string) 
(*adctypes.Resources, error) {
PluginMetadata: metadata,
}, nil
 }
+
+func (s *Store) ListGlobalRules(name string) ([]*adctypes.GlobalRuleItem, 
error) {
+   s.Lock()
+   defer s.Unlock()
+   targetCache, ok := s.cacheMap[name]
+   if !ok {
+   return nil, fmt.Errorf("cache not found for name: %s", name)
+   }
+   globalRules, err := targetCache.ListGlobalRules()
+   if err != nil {
+   return nil, fmt.Errorf("failed to list global rules: %w", err)
+   }
+   return globalRules, nil
+}
+
+func (s *Store) GetResourceLabel(name, resourceType string, id string) 
(map[string]string, error) {
+   s.Lock()
+   defer s.Unlock()
+   targetCache, ok := s.cacheMap[name]
+   if !ok {
+   return nil, fmt.Errorf("cache not found for name: %s", name)
+   }
+   switch resourceType {
+   case "service":

Review Comment:
   fixed.



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


AlinsRan commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188929438


##
internal/controller/utils/utils.go:
##
@@ -0,0 +1,57 @@
+// 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 controller

Review Comment:
   fixed.



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188846868


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }

Review Comment:
   Regarding the handling of `statusUpdateMap` and `d.statusUpdateMap` by this 
function, I think we need to add more comments.



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188846868


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }

Review Comment:
   Regarding the handling of `statusUpdateMap` and `d.statusUpdateMap` by this 
function, I think we need more comments.



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188837845


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+   log.Warnw("sync failed", zap.String("configName", configName), 
zap.Any("statuses", execErrors))
+   for _, execErr := range execErrors.Errors {
+   for _, failedStatus := range execErr.FailedErrors {
+   if len(failedStatus.FailedStatuses) == 0 {
+   resource, err := 
d.store.GetResources(execErr.Name)
+   if err != nil {
+   log.Errorw("failed to get 
resources from store", zap.String("configName", configName), zap.Error(err))
+   continue
+   }
+
+   fillStatusUpdateMapFunc := func(obj 
adctypes.Object) {
+   labels := obj.GetLabels()
+   statusKey := 
types.NamespacedNameKind{
+   Name:  
labels[label.LabelName],
+   Namespace: 
labels[label.LabelNamespace],
+   Kind:  
labels[label.LabelKind],
+   }
+   if msgs, ok := 
statusUpdateMap[statusKey]; ok {
+   
statusUpdateMap[statusKey] = append(msgs, failedStatus.Error())
+   } else {
+   
statusUpdateMap[statusKey] = []string{failedStatus.Error()}
+   }
+   }
+   for _, service := range 
resource.Services {
+   fillStatusUpdateMapFunc(service)
+   }
+
+   for _, consumer := range 
resource.Consumers {
+   
fillStatusUpdateMapFunc(consumer)
+   }
+
+   for _, ssl := range resource.SSLs {
+   fillStatusUpdateMapFunc(ssl)
+   }
+
+   globalRuleItems, err := 
d.store.ListGlobalRules(configName)
+   if err != nil {
+   log.Errorw("failed to list 
global rules", zap.String("configName", configName), zap.Error(err))
+   }
+   for _, globalRule := range 
globalRuleItems {
+   
fillStatusUpdateMapFunc(globalRule)
+   }
+   continue
+   }
+
+   for _, status := range 
failedStatus.FailedStatuses {
+   id := status.Event.ResourceID
+   labels, err := 
d.store.GetResourceLabel(configName, status.Event.

Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188836717


##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+   log.Warnw("sync failed", zap.String("configName", configName), 
zap.Any("statuses", execErrors))
+   for _, execErr := range execErrors.Errors {
+   for _, failedStatus := range execErr.FailedErrors {
+   if len(failedStatus.FailedStatuses) == 0 {
+   resource, err := 
d.store.GetResources(execErr.Name)
+   if err != nil {
+   log.Errorw("failed to get 
resources from store", zap.String("configName", configName), zap.Error(err))
+   continue
+   }
+
+   fillStatusUpdateMapFunc := func(obj 
adctypes.Object) {
+   labels := obj.GetLabels()
+   statusKey := 
types.NamespacedNameKind{
+   Name:  
labels[label.LabelName],
+   Namespace: 
labels[label.LabelNamespace],
+   Kind:  
labels[label.LabelKind],
+   }
+   if msgs, ok := 
statusUpdateMap[statusKey]; ok {
+   
statusUpdateMap[statusKey] = append(msgs, failedStatus.Error())
+   } else {
+   
statusUpdateMap[statusKey] = []string{failedStatus.Error()}
+   }

Review Comment:
   what about append `statusUpdateMap[statusKey]` directly?
   `statusUpdateMap[statusKey] = append(statusUpdateMap[statusKey], 
failedStatus.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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


Copilot commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188825761


##
internal/provider/adc/executor.go:
##
@@ -51,15 +52,26 @@ func (e *DefaultADCExecutor) Execute(ctx context.Context, 
mode string, config ad
 }
 
 func (e *DefaultADCExecutor) runADC(ctx context.Context, mode string, config 
adcConfig, args []string) error {
-   var failedAddrs []string
+   var execErrs = types.ADCExecutionError{

Review Comment:
   In `runADC`, you collect multiple server errors into a single 
`ADCExecutionError` but later the sync logic expects an `ADCExecutionErrors` 
(plural) wrapper. Consider changing `execErrs` to type `ADCExecutionErrors` so 
downstream `errors.As` checks correctly detect all failures.
   ```suggestion
var execErrs = types.ADCExecutionErrors{
   ```



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188815286


##
internal/provider/adc/store.go:
##
@@ -244,3 +245,79 @@ func (s *Store) GetResources(name string) 
(*adctypes.Resources, error) {
PluginMetadata: metadata,
}, nil
 }
+
+func (s *Store) ListGlobalRules(name string) ([]*adctypes.GlobalRuleItem, 
error) {
+   s.Lock()
+   defer s.Unlock()
+   targetCache, ok := s.cacheMap[name]
+   if !ok {
+   return nil, fmt.Errorf("cache not found for name: %s", name)
+   }
+   globalRules, err := targetCache.ListGlobalRules()
+   if err != nil {
+   return nil, fmt.Errorf("failed to list global rules: %w", err)
+   }
+   return globalRules, nil
+}
+
+func (s *Store) GetResourceLabel(name, resourceType string, id string) 
(map[string]string, error) {
+   s.Lock()
+   defer s.Unlock()
+   targetCache, ok := s.cacheMap[name]
+   if !ok {
+   return nil, fmt.Errorf("cache not found for name: %s", name)
+   }
+   switch resourceType {
+   case "service":

Review Comment:
   adctypes.TypeService



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-06 Thread via GitHub


ronething commented on code in PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#discussion_r2188483680


##
internal/controller/utils/utils.go:
##
@@ -0,0 +1,57 @@
+// 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 controller

Review Comment:
   package name can be `utils`?



##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
+   statusUpdateMap := d.resolveADCExecutionErrors(statusesMap)
+   d.handlerStatusUpdate(statusUpdateMap)
+}
+
+func (d *adcClient) handlerStatusUpdate(statusUpdateMap 
map[types.NamespacedNameKind][]string) {
+   for nnk, msgs := range statusUpdateMap {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonSyncFailed,
+   false,
+   0,
+   strings.Join(msgs, "; "),
+   ))
+   }
+
+   for nnk := range d.statusUpdateMap {
+   if _, ok := statusUpdateMap[nnk]; !ok {
+   d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
+   apiv2.ConditionReasonAccepted,
+   true,
+   0,
+   "",
+   ))
+   }
+   }
+   d.statusUpdateMap = statusUpdateMap
+}
+
+func (d *adcClient) resolveADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) map[types.NamespacedNameKind][]string {
+   statusUpdateMap := map[types.NamespacedNameKind][]string{}
+   for configName, execErrors := range statusesMap {
+   log.Warnw("sync failed", zap.String("configName", configName), 
zap.Any("statuses", execErrors))
+   for _, execErr := range execErrors.Errors {
+   for _, failedStatus := range execErr.FailedErrors {

Review Comment:
   Can multi-layer nesting be deconstructed?



##
internal/provider/adc/adc.go:
##
@@ -404,3 +415,150 @@ func prepareSyncFile(resources any) (string, func(), 
error) {
 
return tmpFile.Name(), cleanup, nil
 }
+
+func (d *adcClient) handlerADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {

Review Comment:
   ```suggestion
   func (d *adcClient) handleADCExecutionErrors(statusesMap 
map[string]types.ADCExecutionErrors) {
   ```



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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-05 Thread via GitHub


github-actions[bot] commented on PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#issuecomment-3040869634

   
   
   # conformance test report - apisix mode
   ```yaml
   apiVersion: gateway.networking.k8s.io/v1
   date: "2025-07-06T04:55:26Z"
   gatewayAPIChannel: standard
   gatewayAPIVersion: v1.2.0
   implementation:
 contact: null
 organization: APISIX
 project: apisix-ingress-controller
 url: https://github.com/apache/apisix-ingress-controller.git
 version: v2.0.0
   kind: ConformanceReport
   mode: default
   profiles:
   - core:
   failedTests:
   - HTTPRouteCrossNamespace
   - HTTPRouteInvalidBackendRefUnknownKind
   result: failure
   skippedTests:
   - HTTPRouteHTTPSListener
   statistics:
 Failed: 2
 Passed: 30
 Skipped: 1
 name: GATEWAY-HTTP
 summary: Core tests failed with 2 test failures.
   ```
   


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



Re: [PR] feat: add synchronization status to CRD [apisix-ingress-controller]

2025-07-05 Thread via GitHub


github-actions[bot] commented on PR #2460:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2460#issuecomment-3040868619

   
   
   # conformance test report - apisix-standalone mode
   ```yaml
   apiVersion: gateway.networking.k8s.io/v1
   date: "2025-07-06T04:54:56Z"
   gatewayAPIChannel: standard
   gatewayAPIVersion: v1.2.0
   implementation:
 contact: null
 organization: APISIX
 project: apisix-ingress-controller
 url: https://github.com/apache/apisix-ingress-controller.git
 version: v2.0.0
   kind: ConformanceReport
   mode: default
   profiles:
   - core:
   failedTests:
   - HTTPRouteCrossNamespace
   result: failure
   skippedTests:
   - HTTPRouteHTTPSListener
   statistics:
 Failed: 1
 Passed: 31
 Skipped: 1
 name: GATEWAY-HTTP
 summary: Core tests failed with 1 test failures.
   ```
   


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