Skip to content
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
11 changes: 11 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ version=v${build_date}-${git_commit}

SOURCE_GIT_TAG=v1.0.0+$(shell git rev-parse --short=7 HEAD)

GOLINT=golangci-lint

GO_LD_EXTRAFLAGS=-X github.com/openshift/release-controller/vendor/k8s.io/client-go/pkg/version.gitCommit=$(shell git rev-parse HEAD) -X github.com/openshift/release-controller/vendor/k8s.io/client-go/pkg/version.gitVersion=${SOURCE_GIT_TAG} -X k8s.io/test-infra/prow/version.Name=release-controller -X k8s.io/test-infra/prow/version.Version=${version}
GO_TEST_FLAGS=
export CGO_ENABLED := 0
Expand All @@ -34,6 +36,15 @@ sonar-reports:
golangci-lint run ./... --verbose --no-config --out-format checkstyle --issues-exit-code 0 > golangci-lint.out
.PHONY: sonar-reports

lint:
@echo "Running linter..."
@if command -v $(GOLINT) >/dev/null 2>&1; then \
$(GOLINT) run ./$(BINARY_PATH)/...; \
else \
echo "golangci-lint not installed. Install with: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest"; \
fi
.PHONY: lint

# Legacy targets
image:
imagebuilder -t openshift/release-controller:latest .
Expand Down
59 changes: 57 additions & 2 deletions cmd/release-controller/config_validate.go
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these changes, specifically, to leverage the pre-existing presubmit job, in openshift/release, to validate the new release qualifiers config file.

Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,22 @@ import (
"time"

releasecontroller "github.com/openshift/release-controller/pkg/release-controller"

"github.com/openshift/release-controller/pkg/releasequalifiers"
"gopkg.in/robfig/cron.v2"
"gopkg.in/yaml.v2"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
)

func validateConfigs(configDir string) error {
func validateConfigs(options *options) error {
errors := []error{}
errors = append(errors, validateReleaseConfigs(options.validateConfigs))
if len(options.ReleaseQualifiersConfigPath) > 0 {
errors = append(errors, validateReleaseQualifiersConfig(options.ReleaseQualifiersConfigPath)...)
}
return utilerrors.NewAggregate(errors)
}

func validateReleaseConfigs(configDir string) error {
errors := []error{}
releaseConfigs := []releasecontroller.ReleaseConfig{}
err := filepath.Walk(configDir, func(path string, info os.FileInfo, err error) error {
Expand All @@ -37,8 +47,10 @@ func validateConfigs(configDir string) error {
if err != nil {
return fmt.Errorf("error encountered while trying to read config files: %w", err)
}
errors = append(errors, validateUpgradeJobs(releaseConfigs)...)
errors = append(errors, verifyPeriodicFields(releaseConfigs)...)
errors = append(errors, findDuplicatePeriodics(releaseConfigs)...)
errors = append(errors, validateQualifiers(releaseConfigs)...)
return utilerrors.NewAggregate(errors)
}

Expand Down Expand Up @@ -105,3 +117,46 @@ func findDuplicatePeriodics(releaseConfigs []releasecontroller.ReleaseConfig) []
}
return duplicates
}

// validateQualifiers validates the contents of releasequalifiers.ReleaseQualifiers defined in releasecontroller.ReleaseConfig
func validateQualifiers(releaseConfigs []releasecontroller.ReleaseConfig) []error {
errors := []error{}
for _, config := range releaseConfigs {
for verificationName, verification := range config.Verify {
for qualifierId, overrides := range verification.Qualifiers {
if err := validateQualifier(qualifierId, overrides); err != nil {
errors = append(errors, fmt.Errorf("unable to validate %q release qualifier %s: %v", verificationName, qualifierId, err))
}
}
}
}
return errors
}

// validateReleaseQualifiersConfig validates the contents of the file located at options.ReleaseQualifiersConfigPath
func validateReleaseQualifiersConfig(configPath string) []error {
errors := []error{}
raw, err := os.ReadFile(configPath)
if err != nil {
return errors
}
config := releasecontroller.ReleaseQualifiersConfig{}
dec := yaml.NewDecoder(bytes.NewReader(raw))
dec.SetStrict(true)
if err = dec.Decode(&config); err != nil {
errors = append(errors, fmt.Errorf("failed to unmarshal release qualifier configuration file %s: %v", configPath, err))
return errors
}
for qualifierId, overrides := range config.Qualifiers {
errors = append(errors, validateQualifier(qualifierId, overrides))
}
return errors
}

func validateQualifier(name releasequalifiers.QualifierId, qualifier releasequalifiers.ReleaseQualifier) error {
err := qualifier.Validate()
if err != nil {
return fmt.Errorf("unable to validate qualifier %s: %v", name, err)
}
return nil
}
81 changes: 80 additions & 1 deletion cmd/release-controller/config_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"testing"

releasecontroller "github.com/openshift/release-controller/pkg/release-controller"

"github.com/openshift/release-controller/pkg/releasequalifiers"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
)

Expand Down Expand Up @@ -356,3 +356,82 @@ func TestValidateUpgradeJobs(t *testing.T) {
}
}
}

func TestValidateQualifiersConfiguration(t *testing.T) {
testCases := []struct {
name string
configs []releasecontroller.ReleaseConfig
expectedErr bool
}{
{
name: "Good config with no qualifiers",
configs: []releasecontroller.ReleaseConfig{{
Name: "4.19.0-0.nightly",
Verify: map[string]releasecontroller.ReleaseVerification{
"osd-aws": {
Optional: true,
MaxRetries: 2,
ProwJob: &releasecontroller.ProwJobVerification{
Name: "periodic-ci-openshift-release-master-nightly-4.19-osd-aws",
},
},
},
},
},
expectedErr: false,
},
{
name: "Good config with empty qualifier",
configs: []releasecontroller.ReleaseConfig{{
Name: "4.19.0-0.nightly",
Verify: map[string]releasecontroller.ReleaseVerification{
"osd-aws": {
Optional: true,
MaxRetries: 2,
ProwJob: &releasecontroller.ProwJobVerification{
Name: "periodic-ci-openshift-release-master-nightly-4.19-osd-aws",
},
Qualifiers: releasequalifiers.ReleaseQualifiers{
"rosa": {},
},
},
},
},
},
expectedErr: false,
},
{
name: "Good config with simple overrides",
configs: []releasecontroller.ReleaseConfig{{
Name: "4.19.0-0.nightly",
Verify: map[string]releasecontroller.ReleaseVerification{
"osd-aws": {
Optional: true,
MaxRetries: 2,
ProwJob: &releasecontroller.ProwJobVerification{
Name: "periodic-ci-openshift-release-master-nightly-4.19-osd-aws",
},
Qualifiers: releasequalifiers.ReleaseQualifiers{
"hcm": {
Enabled: releasequalifiers.BoolPtr(false),
BadgeName: "HCM Updated",
Description: "An updated description when displaying badge details",
PayloadBadge: releasequalifiers.PayloadBadgeNo,
},
},
},
},
}},
expectedErr: false,
},
}
for _, testCase := range testCases {
errs := validateQualifiers(testCase.configs)
if len(errs) > 0 && !testCase.expectedErr {
t.Errorf("%s: got error when error was not expected: %v", testCase.name, errs)
}
if len(errs) == 0 && testCase.expectedErr {
t.Errorf("%s: did not get error when error was expected", testCase.name)
}
}
}
12 changes: 6 additions & 6 deletions cmd/release-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,16 @@ func NewController(

c := &Controller{
eventRecorder: recorder,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "releases"),
gcQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "gc"),
queue: workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "releases"}),
gcQueue: workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "gc"}),

// rate limit the audit queue severely
auditQueue: workqueue.NewNamedRateLimitingQueue(workqueue.NewMaxOfRateLimiter(
auditQueue: workqueue.NewRateLimitingQueueWithConfig(workqueue.NewMaxOfRateLimiter(
workqueue.NewItemExponentialFailureRateLimiter(5*time.Second, 2*time.Hour),
&workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Every(5), 2)},
), "audit"),
), workqueue.RateLimitingQueueConfig{Name: "audit"}),

jiraQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "jira"),
jiraQueue: workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "jira"}),

expectations: newExpectations(),
expectationDelay: 2 * time.Second,
Expand Down Expand Up @@ -229,7 +229,7 @@ func NewController(
releasePayloadClient: releasePayloadClient,
releasePayloadLister: &releasecontroller.MultiReleasePayloadLister{Listers: make(map[string]v1alpha1.ReleasePayloadNamespaceLister)},

legacyResultsQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "legacyResults"),
legacyResultsQueue: workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{Name: "legacyResults"}),

manifestListMode: manifestListMode,
}
Expand Down
14 changes: 8 additions & 6 deletions cmd/release-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@ import (
"strings"
"time"

"golang.org/x/text/cases"
"golang.org/x/text/language"

releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions"

releasepayloadclient "github.com/openshift/release-controller/pkg/client/clientset/versioned"
releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions"
"github.com/openshift/release-controller/pkg/jira"
"golang.org/x/text/cases"
"golang.org/x/text/language"

releasecontroller "github.com/openshift/release-controller/pkg/release-controller"

Expand Down Expand Up @@ -112,6 +110,8 @@ type options struct {

ProcessLegacyResults bool
ManifestListMode bool

ReleaseQualifiersConfigPath string
}

// Add metrics for jira verifier errors
Expand Down Expand Up @@ -221,6 +221,8 @@ func main() {
flagset.BoolVar(&opt.ProcessLegacyResults, "process-legacy-results", opt.ProcessLegacyResults, "enable the migration of imagestream based results to ReleasePayloads")
flagset.BoolVar(&opt.ManifestListMode, "manifest-list-mode", opt.ManifestListMode, "enable manifest list support for oc operations")

flagset.StringVar(&opt.ReleaseQualifiersConfigPath, "release-qualifiers-config-path", "", "Path to release qualifiers config file")

goFlagSet := flag.NewFlagSet("prowflags", flag.ContinueOnError)
opt.github.AddFlags(goFlagSet)
opt.jira.AddFlags(goFlagSet)
Expand All @@ -241,7 +243,7 @@ func (o *options) Run() error {
// report liveness on default prow health port 8081
health := pjutil.NewHealthOnPort(flagutil.DefaultHealthPort)
if o.validateConfigs != "" {
return validateConfigs(o.validateConfigs)
return validateConfigs(o)
}
tagParts := strings.Split(o.ToolsImageStreamTag, ":")
if len(tagParts) != 2 || len(tagParts[1]) == 0 {
Expand Down
21 changes: 11 additions & 10 deletions cmd/release-controller/sync_release_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,36 +69,37 @@ func newReleasePayload(release *releasecontroller.Release, name, jobNamespace, p
sort.Strings(sortedKeys)

for _, verifyName := range sortedKeys {
definition := verificationJobs[verifyName]
if definition.Disabled {
verificationJobDefinition := verificationJobs[verifyName]
if verificationJobDefinition.Disabled {
continue
}

ciConfig := v1alpha1.CIConfiguration{
CIConfigurationName: verifyName,
CIConfigurationJobName: definition.ProwJob.Name,
MaxRetries: definition.MaxRetries,
CIConfigurationJobName: verificationJobDefinition.ProwJob.Name,
MaxRetries: verificationJobDefinition.MaxRetries,
}

switch {
case definition.AggregatedProwJob != nil:
case verificationJobDefinition.AggregatedProwJob != nil:
// Every Aggregated Job will contain a Blocking "Aggregator" job and an Informing "Analysis" job
// Adding the Blocking Job
blockingJobName := defaultAggregateProwJobName
if definition.AggregatedProwJob.ProwJob != nil && len(definition.AggregatedProwJob.ProwJob.Name) > 0 {
blockingJobName = definition.AggregatedProwJob.ProwJob.Name
if verificationJobDefinition.AggregatedProwJob.ProwJob != nil && len(verificationJobDefinition.AggregatedProwJob.ProwJob.Name) > 0 {
blockingJobName = verificationJobDefinition.AggregatedProwJob.ProwJob.Name
}
ciConfig.CIConfigurationJobName = fmt.Sprintf("%s-%s", verifyName, blockingJobName)
payload.Spec.PayloadVerificationConfig.BlockingJobs = append(payload.Spec.PayloadVerificationConfig.BlockingJobs, ciConfig)

// Adding the Informing Job
informingJob := v1alpha1.CIConfiguration{
CIConfigurationName: verifyName,
CIConfigurationJobName: definition.ProwJob.Name,
AnalysisJobCount: definition.AggregatedProwJob.AnalysisJobCount,
CIConfigurationJobName: verificationJobDefinition.ProwJob.Name,
AnalysisJobCount: verificationJobDefinition.AggregatedProwJob.AnalysisJobCount,
}
payload.Spec.PayloadVerificationConfig.InformingJobs = append(payload.Spec.PayloadVerificationConfig.InformingJobs, informingJob)
default:
if definition.Optional {
if verificationJobDefinition.Optional {
payload.Spec.PayloadVerificationConfig.InformingJobs = append(payload.Spec.PayloadVerificationConfig.InformingJobs, ciConfig)
} else {
payload.Spec.PayloadVerificationConfig.BlockingJobs = append(payload.Spec.PayloadVerificationConfig.BlockingJobs, ciConfig)
Expand Down
18 changes: 9 additions & 9 deletions cmd/release-controller/sync_release_payload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ var (

func TestNewReleasePayload(t *testing.T) {
testCases := []struct {
name string
release *releasecontroller.Release
payloadName string
jobNamespace string
prowNamespace string
verificationJobs map[string]releasecontroller.ReleaseVerification
upgradeJobs map[string]releasecontroller.UpgradeVerification
dataSource v1alpha1.PayloadVerificationDataSource
expected *v1alpha1.ReleasePayload
name string
release *releasecontroller.Release
payloadName string
jobNamespace string
prowNamespace string
verificationJobs map[string]releasecontroller.ReleaseVerification
upgradeJobs map[string]releasecontroller.UpgradeVerification
dataSource v1alpha1.PayloadVerificationDataSource
expected *v1alpha1.ReleasePayload
}{
{
name: "DisabledJob",
Expand Down
Loading