Skip to content

Commit 8b967c1

Browse files
committed
refactor: Clean up unused variables and update TODO comments in translators
- Removed redundant `apisixUpstreams` and `adcUpstreams` variables in ApisixRoute translator logic for clarity. - Streamlined upstream handling and variable naming in traffic-split logic. - Updated TODO comments to reflect upcoming `.Checks`, `.TLS`, and `.Discovery*` implementations.
1 parent 0e2923f commit 8b967c1

5 files changed

Lines changed: 17 additions & 33 deletions

File tree

docs/crd/api.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,8 +1300,6 @@ _Appears in:_
13001300

13011301

13021302

1303-
1304-
13051303
#### ApisixTlsSpec
13061304

13071305

internal/controller/apisixroute_controller.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ func (r *ApisixRouteReconciler) processApisixRouteHTTPRule(ctx context.Context,
186186
}
187187

188188
// check vars
189-
// todo: cache the result to tctx
190189
if _, err := rule.Match.NginxVars.ToVars(); err != nil {
191190
return ReasonError{
192191
Reason: string(apiv2.ConditionReasonInvalidSpec),

internal/provider/adc/translator/apisixroute.go

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -149,64 +149,50 @@ func (t *Translator) TranslateApisixRoute(tctx *provider.TranslateContext, ar *a
149149
}
150150

151151
var (
152-
apisixUpstreams []*apiv2.ApisixUpstream
153-
adcUpstreams []*adc.Upstream
152+
upstreams []*adc.Upstream
154153
)
155-
data, _ := json.Marshal(rule.Upstreams)
156-
log.Debugf(".http[].Upstreams: %s", data)
157154
for _, upstreamRef := range rule.Upstreams {
158155
upsNN := types.NamespacedName{
159156
Namespace: ar.GetNamespace(),
160157
Name: upstreamRef.Name,
161158
}
162-
log.Debugf("try to get ApisixUpstream: %s", upsNN)
163159
au, ok := tctx.Upstreams[upsNN]
164160
if !ok {
165161
log.Debugf("failed to retrieve ApisixUpstream from tctx, ApisixUpstream: %s", upsNN)
166162
continue
167163
}
168-
log.Debugf("try to translate the ApisixUpstream: %v", au)
169-
adcUpstream, err := t.translateApisixUpstream(tctx, au)
164+
upstream, err := t.translateApisixUpstream(tctx, au)
170165
if err != nil {
171-
log.Debugf("failed to translate ApisixUpstream, ApisixUpstream: %v, err: %v", au, err)
172166
t.Log.Error(err, "failed to translate ApisixUpstream", "ApisixUpstream", utils.NamespacedName(au))
173167
continue
174168
}
175169

176-
apisixUpstreams = append(apisixUpstreams, au)
177-
adcUpstreams = append(adcUpstreams, adcUpstream)
170+
upstreams = append(upstreams, upstream)
178171
}
179172

180-
_ = apisixUpstreams // todo: remove this
181-
182-
data, _ = json.Marshal(adcUpstreams)
183-
log.Debugf("len(rule.Backends): %v, adcUpstreams: %s", len(rule.Backends), string(data))
184-
185173
// If no .http[].backends is used and only .http[].upstreams is used, the first valid upstream is used as service.upstream;
186174
// Other upstreams are configured in the traffic-split plugin
187-
if len(rule.Backends) == 0 && len(adcUpstreams) > 0 {
188-
log.Debugf("set first upstream as service.Upstream, first upstream: %v", adcUpstreams[0])
189-
upstream = adcUpstreams[0]
190-
adcUpstreams = adcUpstreams[1:]
175+
if len(rule.Backends) == 0 && len(upstreams) > 0 {
176+
upstream = upstreams[0]
177+
upstreams = upstreams[1:]
191178
}
192179

193-
var wups []adc.TrafficSplitConfigRuleWeightedUpstream
194-
for _, adcUpstream := range adcUpstreams {
195-
weight, err := strconv.Atoi(adcUpstream.Labels["meta_weight"])
180+
var weightedUpstreams []adc.TrafficSplitConfigRuleWeightedUpstream
181+
for _, item := range upstreams {
182+
weight, err := strconv.Atoi(item.Labels["meta_weight"])
196183
if err != nil {
197-
t.Log.Error(err, "failed to parse meta_weight from upstream labels", "labels", adcUpstream.GetLabels())
198184
weight = apiv2.DefaultWeight
199185
}
200-
wups = append(wups, adc.TrafficSplitConfigRuleWeightedUpstream{
201-
Upstream: adcUpstream,
186+
weightedUpstreams = append(weightedUpstreams, adc.TrafficSplitConfigRuleWeightedUpstream{
187+
Upstream: item,
202188
Weight: weight,
203189
})
204190
}
205-
if len(wups) > 0 {
191+
if len(weightedUpstreams) > 0 {
206192
route.Plugins["traffic-split"] = &adc.TrafficSplitConfig{
207193
Rules: []adc.TrafficSplitConfigRule{
208194
{
209-
WeightedUpstreams: wups,
195+
WeightedUpstreams: weightedUpstreams,
210196
},
211197
},
212198
}

internal/provider/adc/translator/apisixupstream.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func translateApisixUpstreamLoadBalancer(au *apiv2.ApisixUpstream, ups *adc.Upst
9999
}
100100

101101
func translateApisixUpstreamHealthCheck(au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
102-
// todo: no field `.Checks` in adc.Upstream
102+
// todo: handle `.Checks` in next PR
103103
return nil
104104
}
105105

@@ -141,7 +141,7 @@ func translateApisixUpstreamRetriesAndTimeout(au *apiv2.ApisixUpstream, ups *adc
141141
}
142142

143143
func translateApisixUpstreamClientTLS(au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
144-
// todo: no field `.TLS` in adc.Upstream
144+
// todo: handle `.TLS` in next PR
145145
return nil
146146
}
147147

@@ -159,7 +159,7 @@ func translateApisixUpstreamPassHost(au *apiv2.ApisixUpstream, ups *adc.Upstream
159159
}
160160

161161
func translateApisixUpstreamDiscovery(upstream *apiv2.ApisixUpstream, upstream2 *adc.Upstream) error {
162-
// todo: no filed `.Discovery*` in adc.Upstream
162+
// todo: handle `.Discovery*` in next PR
163163
return nil
164164
}
165165

internal/utils/k8s.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ var hostDefRegex = regexp.MustCompile(hostDef)
5656
// ref to : https://github.com/apache/apisix/blob/c5fc10d9355a0c177a7532f01c77745ff0639a7f/apisix/schema_def.lua#L40
5757
// ref to : https://github.com/kubernetes/kubernetes/blob/976a940f4a4e84fe814583848f97b9aafcdb083f/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L205
5858
// They define regex differently, but k8s's dns is more accurate
59+
// todo: validate by CRD definition
5960
func MatchHostDef(host string) bool {
6061
return hostDefRegex.MatchString(host)
6162
}

0 commit comments

Comments
 (0)