Skip to content

Commit

Permalink
Merge pull request openshift#993 from petr-muller/rehearse-cm-code-cl…
Browse files Browse the repository at this point in the history 
…eanups

pj-rehearse code cleanups
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Jul 9, 2020
2 parents fd8861e + c17675e commit 075b4a2
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 71 deletions.
12 changes: 6 additions & 6 deletions cmd/pj-rehearse/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pj-rehearse created invalid rehearsal jobs.This is either a pj-rehearse bug, or
the rehearsed jobs themselves are invalid.`
)

func loadPluginConfig(releaseRepoPath string) (ret prowplugins.ConfigUpdater, err error) {
func loadConfigUpdaterCfg(releaseRepoPath string) (ret prowplugins.ConfigUpdater, err error) {
agent := prowplugins.ConfigAgent{}
if err = agent.Load(filepath.Join(releaseRepoPath, config.PluginConfigInRepoPath), true); err == nil {
ret = agent.Config().ConfigUpdater
Expand Down Expand Up @@ -183,7 +183,7 @@ func rehearseMain() error {
}

prConfig := config.GetAllConfigs(o.releaseRepoPath, logger)
pluginConfig, err := loadPluginConfig(o.releaseRepoPath)
configUpdaterCfg, err := loadConfigUpdaterCfg(o.releaseRepoPath)
if err != nil {
logger.WithError(err).Error("could not load plugin configuration from tested revision of release repo")
return fmt.Errorf(misconfigurationOutput)
Expand Down Expand Up @@ -310,7 +310,7 @@ func rehearseMain() error {
toRehearseClusterProfiles := diffs.GetPresubmitsForClusterProfiles(prConfig.Prow, changedClusterProfiles, logger)
toRehearse.AddAll(toRehearseClusterProfiles)

presubmitsWithChangedRegistry := rehearse.AddRandomJobsForChangedRegistry(changedRegistrySteps, graph, prConfig.Prow.JobConfig.PresubmitsStatic, filepath.Join(o.releaseRepoPath, config.CiopConfigInRepoPath), loggers)
presubmitsWithChangedRegistry := rehearse.AddRandomJobsForChangedRegistry(changedRegistrySteps, prConfig.Prow.JobConfig.PresubmitsStatic, filepath.Join(o.releaseRepoPath, config.CiopConfigInRepoPath), loggers)
toRehearse.AddAll(presubmitsWithChangedRegistry)

resolver := registry.NewResolver(refs, chains, workflows)
Expand Down Expand Up @@ -373,7 +373,7 @@ func rehearseMain() error {
prConfig.Prow.ProwJobNamespace,
pjclient,
prConfig.Prow.PodNamespace,
pluginConfig,
configUpdaterCfg,
o.releaseRepoPath,
o.dryRun,
changedTemplates,
Expand Down Expand Up @@ -433,7 +433,7 @@ func setupDependencies(
prowJobNamespace string,
prowJobClient ctrlruntimeclient.Client,
podNamespace string,
pluginConfig prowplugins.ConfigUpdater,
configUpdaterCfg prowplugins.ConfigUpdater,
releaseRepoPath string,
dryRun bool,
changedTemplates []config.ConfigMapSource,
Expand Down Expand Up @@ -477,7 +477,7 @@ func setupDependencies(
return errors.New(misconfigurationOutput)
}

cmManager := config.NewTemplateCMManager(prowJobNamespace, cmClient, pluginConfig, prNumber, releaseRepoPath, log)
cmManager := config.NewRehearsalCMManager(prowJobNamespace, cmClient, configUpdaterCfg, prNumber, releaseRepoPath, log)

cleanupsLock.Lock()
cleanups = append(cleanups, func() {
Expand Down
10 changes: 5 additions & 5 deletions pkg/config/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func GetChangedTemplates(path, baseRev string) ([]ConfigMapSource, error) {
}
var ret []ConfigMapSource
for _, c := range changes {
if filepath.Ext(c.Filename) == ".yaml" {
if filepath.Ext(c.PathInRepo) == ".yaml" {
ret = append(ret, c)
}
}
Expand Down Expand Up @@ -195,8 +195,8 @@ func GetChangedRegistrySteps(path, baseRev string, graph registry.NodeByName) ([
return changes, err
}
for _, c := range revChanges {
if filepath.Ext(c.Filename) == ".yaml" || strings.HasSuffix(c.Filename, "-commands.sh") {
node, err := loadRegistryStep(filepath.Base(c.Filename), graph)
if filepath.Ext(c.PathInRepo) == ".yaml" || strings.HasSuffix(c.PathInRepo, "-commands.sh") {
node, err := loadRegistryStep(filepath.Base(c.PathInRepo), graph)
if err != nil {
return changes, err
}
Expand Down Expand Up @@ -227,8 +227,8 @@ func getRevChanges(root, path, base string, rec bool) ([]ConfigMapSource, error)
var ret []ConfigMapSource
for _, l := range strings.Split(strings.TrimSpace(diff), "\n") {
ret = append(ret, ConfigMapSource{
Filename: filepath.Join(path, l[99:]),
SHA: l[56:96],
PathInRepo: filepath.Join(path, l[99:]),
SHA: l[56:96],
})
}
return ret, nil
Expand Down
28 changes: 14 additions & 14 deletions pkg/config/release_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ func TestGetChangedTemplates(t *testing.T) {
> org/repo/README.md
`
expected := []ConfigMapSource{{
SHA: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
Filename: filepath.Join(TemplatesPath, "cluster-launch-top-level.yaml"),
SHA: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
PathInRepo: filepath.Join(TemplatesPath, "cluster-launch-top-level.yaml"),
}, {
SHA: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
Filename: filepath.Join(TemplatesPath, "org/repo/cluster-launch-subdir.yaml"),
SHA: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
PathInRepo: filepath.Join(TemplatesPath, "org/repo/cluster-launch-subdir.yaml"),
}}
compareChanges(t, TemplatesPath, files, cmd, GetChangedTemplates, expected)
}
Expand All @@ -100,20 +100,20 @@ git mv renameme/file renamed/file
> dir/dir/file
`
expected := []ConfigMapSource{{
SHA: "df2b8fc99e1c1d4dbc0a854d9f72157f1d6ea078",
Filename: filepath.Join(ClusterProfilesPath, "changeme"),
SHA: "df2b8fc99e1c1d4dbc0a854d9f72157f1d6ea078",
PathInRepo: filepath.Join(ClusterProfilesPath, "changeme"),
}, {
SHA: "b4c3cc91598b6469bf7036502b8ca2bd563b0d0a",
Filename: filepath.Join(ClusterProfilesPath, "dir"),
SHA: "b4c3cc91598b6469bf7036502b8ca2bd563b0d0a",
PathInRepo: filepath.Join(ClusterProfilesPath, "dir"),
}, {
SHA: "03b9d461447abb84264053a440b4c715842566bb",
Filename: filepath.Join(ClusterProfilesPath, "moveme"),
SHA: "03b9d461447abb84264053a440b4c715842566bb",
PathInRepo: filepath.Join(ClusterProfilesPath, "moveme"),
}, {
SHA: "df2b8fc99e1c1d4dbc0a854d9f72157f1d6ea078",
Filename: filepath.Join(ClusterProfilesPath, "new"),
SHA: "df2b8fc99e1c1d4dbc0a854d9f72157f1d6ea078",
PathInRepo: filepath.Join(ClusterProfilesPath, "new"),
}, {
SHA: "9bbab5dcf83793f9edc258136426678cccce940e",
Filename: filepath.Join(ClusterProfilesPath, "renamed"),
SHA: "9bbab5dcf83793f9edc258136426678cccce940e",
PathInRepo: filepath.Join(ClusterProfilesPath, "renamed"),
}}
compareChanges(t, ClusterProfilesPath, files, cmd, GetChangedClusterProfiles, expected)
}
30 changes: 15 additions & 15 deletions pkg/config/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ import (
)

type ConfigMapSource struct {
Filename, SHA string
PathInRepo, SHA string
}

func (s ConfigMapSource) Name() string {
base := filepath.Base(s.Filename)
base := filepath.Base(s.PathInRepo)
return strings.TrimSuffix(base, filepath.Ext(base))
}

Expand All @@ -50,8 +50,8 @@ const (
rehearseLabelPull = "ci.openshift.org/rehearse-pull"
)

// TemplateCMManager holds the details needed for the configmap controller
type TemplateCMManager struct {
// RehearsalCMManager holds the details needed for the configmap controller
type RehearsalCMManager struct {
namespace string
cmclient corev1.ConfigMapInterface
configUpdaterCfg prowplugins.ConfigUpdater
Expand All @@ -60,16 +60,16 @@ type TemplateCMManager struct {
logger *logrus.Entry
}

// NewTemplateCMManager creates a new TemplateCMManager
func NewTemplateCMManager(
// NewRehearsalCMManager creates a new RehearsalCMManager
func NewRehearsalCMManager(
namespace string,
cmclient corev1.ConfigMapInterface,
configUpdaterCfg prowplugins.ConfigUpdater,
prNumber int,
releaseRepoPath string,
logger *logrus.Entry,
) *TemplateCMManager {
return &TemplateCMManager{
) *RehearsalCMManager {
return &RehearsalCMManager{
namespace: namespace,
cmclient: cmclient,
configUpdaterCfg: configUpdaterCfg,
Expand All @@ -87,7 +87,7 @@ func (g osFileGetter) GetFile(filename string) ([]byte, error) {
return ioutil.ReadFile(filepath.Join(g.root, filename))
}

func (c *TemplateCMManager) createCM(name string, data []updateconfig.ConfigMapUpdate) error {
func (c *RehearsalCMManager) createCM(name string, data []updateconfig.ConfigMapUpdate) error {
cm := &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -109,7 +109,7 @@ func (c *TemplateCMManager) createCM(name string, data []updateconfig.ConfigMapU
func genChanges(root string, sources []ConfigMapSource) ([]prowgithub.PullRequestChange, error) {
var ret []prowgithub.PullRequestChange
for _, f := range sources {
err := filepath.Walk(filepath.Join(root, f.Filename), func(path string, info os.FileInfo, err error) error {
err := filepath.Walk(filepath.Join(root, f.PathInRepo), func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() {
return err
}
Expand All @@ -131,7 +131,7 @@ func genChanges(root string, sources []ConfigMapSource) ([]prowgithub.PullReques
return ret, nil
}

func (c *TemplateCMManager) validateChanges(changes []prowgithub.PullRequestChange) error {
func (c *RehearsalCMManager) validateChanges(changes []prowgithub.PullRequestChange) error {
var errs []error
for _, change := range changes {
found := false
Expand Down Expand Up @@ -166,7 +166,7 @@ func replaceSpecNames(namespace string, cfg prowplugins.ConfigUpdater, mapping m
return
}

func (c *TemplateCMManager) createCMs(sources []ConfigMapSource, mapping map[string]string) error {
func (c *RehearsalCMManager) createCMs(sources []ConfigMapSource, mapping map[string]string) error {
changes, err := genChanges(c.releaseRepoPath, sources)
if err != nil {
return err
Expand All @@ -185,15 +185,15 @@ func (c *TemplateCMManager) createCMs(sources []ConfigMapSource, mapping map[str
}

// CreateCMTemplates creates configMaps for all the changed templates.
func (c *TemplateCMManager) CreateCMTemplates(templates []ConfigMapSource) error {
func (c *RehearsalCMManager) CreateCMTemplates(templates []ConfigMapSource) error {
nameMap := make(map[string]string, len(templates))
for _, t := range templates {
nameMap[t.CMName(TemplatePrefix)] = t.TempCMName("template")
}
return c.createCMs(templates, nameMap)
}

func (c *TemplateCMManager) CreateClusterProfiles(profiles []ConfigMapSource) error {
func (c *RehearsalCMManager) CreateClusterProfiles(profiles []ConfigMapSource) error {
nameMap := make(map[string]string, len(profiles))
for _, p := range profiles {
nameMap[p.CMName(ClusterProfilePrefix)] = p.TempCMName("cluster-profile")
Expand All @@ -202,7 +202,7 @@ func (c *TemplateCMManager) CreateClusterProfiles(profiles []ConfigMapSource) er
}

// CleanupCMTemplates deletes all the configMaps that have been created for the changed templates.
func (c *TemplateCMManager) CleanupCMTemplates() error {
func (c *RehearsalCMManager) CleanupCMTemplates() error {
c.logger.Info("deleting temporary template configMaps")
if err := c.cmclient.DeleteCollection(&metav1.DeleteOptions{},
metav1.ListOptions{LabelSelector: fields.Set{
Expand Down
38 changes: 19 additions & 19 deletions pkg/config/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestValidateConfigMaps(t *testing.T) {
}
client := fake.NewSimpleClientset().CoreV1().ConfigMaps("ns")
config.SetDefaults()
manager := NewTemplateCMManager("ns", client, config, 0, "/", logrus.NewEntry(logrus.New()))
manager := NewRehearsalCMManager("ns", client, config, 0, "/", logrus.NewEntry(logrus.New()))
if err := manager.validateChanges(changes); err != nil {
t.Errorf("unexpected error: %v", err)
}
Expand All @@ -60,8 +60,8 @@ func TestCreateCleanupCMTemplates(t *testing.T) {
testTemplatePath := filepath.Join(TemplatesPath, "subdir/test-template.yaml")
ns := "test-namespace"
ciTemplates := []ConfigMapSource{{
Filename: testTemplatePath,
SHA: "hd9sxk615lkcwx2kj226g3r3lvwkftyjif2pczm5dq3l0h13p35t",
PathInRepo: testTemplatePath,
SHA: "hd9sxk615lkcwx2kj226g3r3lvwkftyjif2pczm5dq3l0h13p35t",
}}
contents, err := ioutil.ReadFile(filepath.Join(testRepoPath, testTemplatePath))
if err != nil {
Expand Down Expand Up @@ -95,16 +95,16 @@ func TestCreateCleanupCMTemplates(t *testing.T) {
cs := fake.NewSimpleClientset()
cs.Fake.PrependReactor("delete-collection", "configmaps", func(action coretesting.Action) (bool, runtime.Object, error) {
deleteAction := action.(coretesting.DeleteCollectionAction)
listRestricitons := deleteAction.GetListRestrictions()
listRestrictions := deleteAction.GetListRestrictions()

if !reflect.DeepEqual(listRestricitons.Labels, expectedListRestricitons.Labels) {
t.Fatalf("Labels:\nExpected:%#v\nFound: %#v", expectedListRestricitons.Labels, listRestricitons.Labels)
if !reflect.DeepEqual(listRestrictions.Labels, expectedListRestricitons.Labels) {
t.Fatalf("Labels:\nExpected:%#v\nFound: %#v", expectedListRestricitons.Labels, listRestrictions.Labels)
}

return true, nil, nil
})
client := cs.CoreV1().ConfigMaps(ns)
cmManager := NewTemplateCMManager(ns, client, configUpdaterCfg, 1234, testRepoPath, logrus.NewEntry(logrus.New()))
cmManager := NewRehearsalCMManager(ns, client, configUpdaterCfg, 1234, testRepoPath, logrus.NewEntry(logrus.New()))
if err := cmManager.CreateCMTemplates(ciTemplates); err != nil {
t.Fatalf("CreateCMTemplates() returned error: %v", err)
}
Expand Down Expand Up @@ -140,21 +140,21 @@ func TestCreateClusterProfiles(t *testing.T) {
}
defer os.RemoveAll(dir)
profiles := []ConfigMapSource{{
SHA: "e92d4a5996a8a977bd7916b65488371331681f9d",
Filename: filepath.Join(ClusterProfilesPath, "profile0"),
SHA: "e92d4a5996a8a977bd7916b65488371331681f9d",
PathInRepo: filepath.Join(ClusterProfilesPath, "profile0"),
}, {
SHA: "a8c99ffc996128417ef1062f9783730a8c864586",
Filename: filepath.Join(ClusterProfilesPath, "profile1"),
SHA: "a8c99ffc996128417ef1062f9783730a8c864586",
PathInRepo: filepath.Join(ClusterProfilesPath, "profile1"),
}, {
SHA: "8012ff51a005eaa8ed8f4c08ccdce580f462fff6",
Filename: filepath.Join(ClusterProfilesPath, "unchanged"),
SHA: "8012ff51a005eaa8ed8f4c08ccdce580f462fff6",
PathInRepo: filepath.Join(ClusterProfilesPath, "unchanged"),
}}
for _, p := range profiles {
path := filepath.Join(dir, p.Filename)
path := filepath.Join(dir, p.PathInRepo)
if err := os.MkdirAll(path, 0775); err != nil {
t.Fatal(err)
}
content := []byte(filepath.Base(p.Filename) + " content")
content := []byte(filepath.Base(p.PathInRepo) + " content")
if err := ioutil.WriteFile(filepath.Join(path, "file"), content, 0664); err != nil {
t.Fatal(err)
}
Expand All @@ -181,17 +181,17 @@ func TestCreateClusterProfiles(t *testing.T) {
configUpdaterCfg.SetDefaults()
cs := fake.NewSimpleClientset()
client := cs.CoreV1().ConfigMaps(ns)
m := NewTemplateCMManager(ns, client, configUpdaterCfg, pr, dir, logrus.NewEntry(logrus.New()))
m := NewRehearsalCMManager(ns, client, configUpdaterCfg, pr, dir, logrus.NewEntry(logrus.New()))
if err := m.CreateClusterProfiles(profiles); err != nil {
t.Fatal(err)
}
cms, err := client.List(metav1.ListOptions{})
sort.Slice(cms.Items, func(i, j int) bool {
return cms.Items[i].Name < cms.Items[j].Name
})
if err != nil {
t.Fatal(err)
}
sort.Slice(cms.Items, func(i, j int) bool {
return cms.Items[i].Name < cms.Items[j].Name
})
expected := []v1.ConfigMap{{
ObjectMeta: metav1.ObjectMeta{
Name: "rehearse-cluster-profile-profile0-e92d4a59",
Expand Down
10 changes: 5 additions & 5 deletions pkg/diffs/diffs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ func TestGetPresubmitsForClusterProfiles(t *testing.T) {
id: "empty",
cfg: &prowconfig.Config{},
profiles: []config.ConfigMapSource{{
Filename: filepath.Join(config.ClusterProfilesPath, "test-profile"),
PathInRepo: filepath.Join(config.ClusterProfilesPath, "test-profile"),
}},
}, {
id: "not a kubernetes job",
Expand All @@ -752,7 +752,7 @@ func TestGetPresubmitsForClusterProfiles(t *testing.T) {
},
},
profiles: []config.ConfigMapSource{{
Filename: filepath.Join(config.ClusterProfilesPath, "test-profile"),
PathInRepo: filepath.Join(config.ClusterProfilesPath, "test-profile"),
}},
}, {
id: "job doesn't use cluster profiles",
Expand All @@ -766,7 +766,7 @@ func TestGetPresubmitsForClusterProfiles(t *testing.T) {
},
},
profiles: []config.ConfigMapSource{{
Filename: filepath.Join(config.ClusterProfilesPath, "test-profile"),
PathInRepo: filepath.Join(config.ClusterProfilesPath, "test-profile"),
}},
}, {
id: "job doesn't use the cluster profile",
Expand All @@ -780,7 +780,7 @@ func TestGetPresubmitsForClusterProfiles(t *testing.T) {
},
},
profiles: []config.ConfigMapSource{{
Filename: filepath.Join(config.ClusterProfilesPath, "test-profile"),
PathInRepo: filepath.Join(config.ClusterProfilesPath, "test-profile"),
}},
}, {
id: "multiple jobs, one uses cluster the profile",
Expand All @@ -798,7 +798,7 @@ func TestGetPresubmitsForClusterProfiles(t *testing.T) {
},
},
profiles: []config.ConfigMapSource{{
Filename: filepath.Join(config.ClusterProfilesPath, "test-profile"),
PathInRepo: filepath.Join(config.ClusterProfilesPath, "test-profile"),
}},
expected: []string{"uses-cluster-profile"},
}} {
Expand Down
Loading

0 comments on commit 075b4a2

Please sign in to comment.