From fb038a0220a94595deb28136b8810a2cea9b530e Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Fri, 2 Feb 2024 12:11:45 -0800 Subject: [PATCH 1/2] feat: dont error out on version parse if we fail, just skip dealing w/ license secret --- api/v1/srl_version.go | 8 ++++---- api/v1/srl_version_test.go | 10 ++-------- api/v1/srlinux_types.go | 2 +- api/v1/srlinux_types_test.go | 11 +++-------- controllers/secret.go | 13 +++++++++---- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/api/v1/srl_version.go b/api/v1/srl_version.go index b44e162..f9d86cf 100644 --- a/api/v1/srl_version.go +++ b/api/v1/srl_version.go @@ -22,12 +22,12 @@ type SrlVersion struct { Commit string `json:"commit,omitempty"` } -func parseVersionString(s string) (*SrlVersion, error) { +func parseVersionString(s string) *SrlVersion { // Check if the version string is an engineering build with major = 0 engineeringVersions := []string{"", "latest", "ga"} for _, ver := range engineeringVersions { if ver == strings.ToLower(s) { - return &SrlVersion{"0", "", "", "", ""}, nil + return &SrlVersion{"0", "", "", "", ""} } } @@ -38,8 +38,8 @@ func parseVersionString(s string) (*SrlVersion, error) { v := re.FindStringSubmatch(s) if v == nil { - return nil, ErrVersionParse + return &SrlVersion{"0", "", "", "", ""} } - return &SrlVersion{v[1], v[2], v[3], v[4], v[5]}, nil + return &SrlVersion{v[1], v[2], v[3], v[4], v[5]} } diff --git a/api/v1/srl_version_test.go b/api/v1/srl_version_test.go index 3d99379..3884057 100644 --- a/api/v1/srl_version_test.go +++ b/api/v1/srl_version_test.go @@ -5,7 +5,6 @@ package v1 import ( - "errors" "testing" "github.com/google/go-cmp/cmp" @@ -16,7 +15,6 @@ func TestParseVersionString(t *testing.T) { desc string got string want *SrlVersion - err error }{ { desc: "maj, minor, patch", @@ -86,17 +84,13 @@ func TestParseVersionString(t *testing.T) { { desc: "invalid1", got: "abcd", - want: nil, - err: ErrVersionParse, + want: &SrlVersion{"0", "", "", "", ""}, }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - ver, err := parseVersionString(tt.got) - if !errors.Is(err, tt.err) { - t.Fatalf("got error '%v' but expected '%v'", err, tt.err) - } + ver := parseVersionString(tt.got) if !cmp.Equal(ver, tt.want) { t.Fatalf( diff --git a/api/v1/srlinux_types.go b/api/v1/srlinux_types.go index 330a07a..9f91058 100644 --- a/api/v1/srlinux_types.go +++ b/api/v1/srlinux_types.go @@ -159,7 +159,7 @@ func (s *SrlinuxSpec) GetImage() string { // When Version field is set it is returned. // In other cases, Image string is evaluated and it's tag substring is parsed. // If no tag is present, or tag is latest, the 0.0 version is assumed to be in use. -func (s *SrlinuxSpec) GetImageVersion() (*SrlVersion, error) { +func (s *SrlinuxSpec) GetImageVersion() *SrlVersion { if s.Version != "" { return parseVersionString(s.Version) } diff --git a/api/v1/srlinux_types_test.go b/api/v1/srlinux_types_test.go index 892ec99..653f508 100644 --- a/api/v1/srlinux_types_test.go +++ b/api/v1/srlinux_types_test.go @@ -6,7 +6,6 @@ package v1 import ( "context" - "errors" "testing" "github.com/google/go-cmp/cmp" @@ -98,7 +97,7 @@ func TestGetImageVersion(t *testing.T) { Version: "abc", Config: &NodeConfig{Image: "ghcr.io/nokia/srlinux:somever"}, }, - err: ErrVersionParse, + want: &SrlVersion{"0", "", "", "", ""}, }, { desc: "version is not present, valid image tag is given", @@ -112,17 +111,13 @@ func TestGetImageVersion(t *testing.T) { spec: &SrlinuxSpec{ Config: &NodeConfig{Image: "ghcr.io/nokia/srlinux:somesrl"}, }, - err: ErrVersionParse, + want: &SrlVersion{"0", "", "", "", ""}, }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - v, err := tt.spec.GetImageVersion() - - if !errors.Is(err, tt.err) { - t.Fatalf("got error '%v' but expected '%v'", err, tt.err) - } + v := tt.spec.GetImageVersion() if !cmp.Equal(v, tt.want) { t.Fatalf( diff --git a/controllers/secret.go b/controllers/secret.go index 18849fa..1b5285c 100644 --- a/controllers/secret.go +++ b/controllers/secret.go @@ -31,9 +31,14 @@ func (r *SrlinuxReconciler) createSecrets( return err } - v, err := s.Spec.GetImageVersion() - if err != nil { - return err + v := s.Spec.GetImageVersion() + + if v.Major == "0" { + log.Info( + "SR Linux image version could not be parsed, will continue without handling license", + ) + + return nil } log.Info("SR Linux image version parsed", "version", v) @@ -41,7 +46,7 @@ func (r *SrlinuxReconciler) createSecrets( // set license key matching image version s.InitLicenseKey(ctx, secret, v) - return err + return nil } func (r *SrlinuxReconciler) addOrUpdateLicenseSecret( From 1cd1e8552e042a81900bd775909ad250454c3a39 Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Fri, 2 Feb 2024 12:15:13 -0800 Subject: [PATCH 2/2] chore: remove now unneeded version parse error --- api/v1/srl_version.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/api/v1/srl_version.go b/api/v1/srl_version.go index f9d86cf..754dcb7 100644 --- a/api/v1/srl_version.go +++ b/api/v1/srl_version.go @@ -5,14 +5,10 @@ package v1 import ( - "errors" "regexp" "strings" ) -// ErrVersionParse is an error which is raised when srlinux version is failed to parse. -var ErrVersionParse = errors.New("version parsing failed") - // SrlVersion represents an sr linux version as a set of fields. type SrlVersion struct { Major string `json:"major,omitempty"`