Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unmarshal apisix/upstream field nodes be null #724

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

chzhuo
Copy link
Contributor

@chzhuo chzhuo commented Oct 27, 2021

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • Bugfix

  • New feature provided

  • Improve performance

  • Backport patches

  • Related issues


Bugfix

  • Description
    image
    I found this error in apisix-ingress-controller log

Controller sync the upstream from APISIX will set the v1.Upstream.Nodes = nil when the APISIX upstream json like "nodes": {}

we can repeat this:

package main

import (
	"encoding/json"
	"fmt"

	v1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
)

func main() {
	data := `{"node":{}}`
	var ups v1.Upstream

	err := json.Unmarshal([]byte(data), &ups)
	if err != nil {
		panic(err)
	}
	fmt.Println(ups.Nodes == nil) // output: true
}

Output should be false, but is true

@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2021

Codecov Report

Merging #724 (4de1e05) into master (2a73216) will not change coverage.
The diff coverage is n/a.

❗ Current head 4de1e05 differs from pull request most recent head 973bd07. Consider uploading reports for the commit 973bd07 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #724   +/-   ##
=======================================
  Coverage   32.61%   32.61%           
=======================================
  Files          65       65           
  Lines        6500     6500           
=======================================
  Hits         2120     2120           
  Misses       4136     4136           
  Partials      244      244           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a73216...973bd07. Read the comment docs.

@tokers tokers changed the title Fix: unmarshal apisix/upstream field nodes be null fix: unmarshal apisix/upstream field nodes be null Oct 27, 2021
@tao12345666333 tao12345666333 merged commit dc196ef into apache:master Oct 27, 2021
@tao12345666333
Copy link
Member

Thanks!

@tao12345666333 tao12345666333 added this to the 1.4.0 milestone Oct 27, 2021
@tao12345666333 tao12345666333 added this to In progress in v1.4 Planning via automation Oct 27, 2021
@tao12345666333 tao12345666333 added the bug Something isn't working label Oct 27, 2021
@chzhuo chzhuo deleted the fix_apisix-nodes-null branch October 27, 2021 12:40
@tao12345666333 tao12345666333 moved this from In progress to Done in v1.4 Planning Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants