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

[release-4.17] OCPBUGS-41914: populate the endpoint parameter when there's an error #998

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/gatherers/conditional/conditional_gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,19 @@ func (g *Gatherer) getRemoteConfiguration(ctx context.Context) ([]byte, error) {
return nil, err
}

ocpVersion := os.Getenv("RELEASE_VERSION")
ocpVersion, ok := os.LookupEnv("RELEASE_VERSION")
if !ok || ocpVersion == "" {
return nil, fmt.Errorf("environmental variable RELEASE_VERSION is not set or has empty value")
}

backOff := wait.Backoff{
Duration: 30 * time.Second,
Factor: 2,
Jitter: 0,
Steps: 3,
Cap: 3 * time.Minute,
}
endpointWithVersion := fmt.Sprintf(endpoint, ocpVersion)
var remoteConfigData []byte
err = wait.ExponentialBackoffWithContext(ctx, backOff, func(ctx context.Context) (done bool, err error) {
resp, err := g.insightsCli.GetWithPathParam(ctx, endpoint, ocpVersion, false)
Expand All @@ -291,7 +296,7 @@ func (g *Gatherer) getRemoteConfiguration(ctx context.Context) ([]byte, error) {
return false, nil
}
return true, insightsclient.HttpError{
Err: fmt.Errorf("received HTTP %s from %s", resp.Status, endpoint),
Err: fmt.Errorf("received HTTP %s from %s", resp.Status, endpointWithVersion),
StatusCode: resp.StatusCode,
}
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/gatherers/conditional/conditional_gatherer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
)

func Test_Gatherer_Basic(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gatherer := newEmptyGatherer("", "")

assert.Equal(t, "conditional", gatherer.GetName())
Expand All @@ -32,6 +33,7 @@ func Test_Gatherer_Basic(t *testing.T) {
}

func Test_Gatherer_GetGatheringFunctions(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gatherer := newEmptyGatherer("", "")

ctx := context.Background()
Expand All @@ -46,6 +48,7 @@ func Test_Gatherer_GetGatheringFunctions(t *testing.T) {
}

func Test_Gatherer_GetGatheringFunctions_BuiltInConfigIsUsed(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gatherer := newEmptyGatherer("", "")

ctx := context.Background()
Expand Down Expand Up @@ -73,6 +76,7 @@ func Test_Gatherer_GetGatheringFunctions_BuiltInConfigIsUsed(t *testing.T) {
}

func Test_Gatherer_GetGatheringFunctions_InvalidConfig(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gathererConfig := `{
"version": "1.0.0",
"conditional_gathering_rules": [{
Expand Down Expand Up @@ -107,6 +111,7 @@ func Test_Gatherer_GetGatheringFunctions_InvalidConfig(t *testing.T) {
}

func Test_Gatherer_GetGatheringFunctions_NoConditionsAreSatisfied(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gatherer := newEmptyGatherer("", "")

gatheringFunctions, err := gatherer.GetGatheringFunctions(context.Background())
Expand All @@ -118,6 +123,7 @@ func Test_Gatherer_GetGatheringFunctions_NoConditionsAreSatisfied(t *testing.T)
}

func Test_Gatherer_GetGatheringFunctions_ConditionIsSatisfied(t *testing.T) {
t.Setenv("RELEASE_VERSION", "1.2.3")
gatherer := newEmptyGatherer("", "")

ctx := context.Background()
Expand Down Expand Up @@ -281,19 +287,22 @@ func TestGetGatheringFunctions(t *testing.T) {
name string
endpoint string
remoteConfig string
releaseVersionEnvVar string
expectedErrMsg string
expectRemoteConfigErr bool
}{
{
name: "remote configuration is available and can be parsed",
endpoint: "/gathering_rules",
releaseVersionEnvVar: "1.2.3",
remoteConfig: "",
expectedErrMsg: "",
expectRemoteConfigErr: false,
},
{
name: "remote configuration is not available",
endpoint: "not valid endpoint",
releaseVersionEnvVar: "1.2.3",
remoteConfig: "",
expectedErrMsg: "endpoint not supported",
expectRemoteConfigErr: true,
Expand All @@ -302,13 +311,23 @@ func TestGetGatheringFunctions(t *testing.T) {
name: "remote configuration is available, but cannot be parsed",
endpoint: "/gathering_rules",
remoteConfig: `{not json}`,
releaseVersionEnvVar: "1.2.3",
expectedErrMsg: "invalid character 'n' looking for beginning of object key string",
expectRemoteConfigErr: true,
},
{
name: "remote configuration is not available, because RELEASE_VERSION is set with empty",
endpoint: "/gathering_rules",
releaseVersionEnvVar: "",
remoteConfig: "",
expectedErrMsg: "environmental variable RELEASE_VERSION is not set or has empty value",
expectRemoteConfigErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("RELEASE_VERSION", tt.releaseVersionEnvVar)
gatherer := newEmptyGatherer(tt.remoteConfig, tt.endpoint)
_, err := gatherer.GetGatheringFunctions(context.Background())
if err != nil {
Expand Down