Skip to content

Commit

Permalink
pay my respects to the all-knowing linters
Browse files Browse the repository at this point in the history
Signed-off-by: davis-haba <davishaba@google.com>
  • Loading branch information
davis-haba committed Apr 18, 2023
1 parent c03ec54 commit 2357e4a
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 31 deletions.
12 changes: 8 additions & 4 deletions pkg/controller/expansion/expansion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ type Reconciler struct {
func newReconciler(mgr manager.Manager,
system *expansion.System,
getPod func(ctx context.Context) (*corev1.Pod, error),
tracker *readiness.Tracker) *Reconciler {
tracker *readiness.Tracker,
) *Reconciler {
ev := make(chan event.GenericEvent, eventQueueSize)
return &Reconciler{
Client: mgr.GetClient(),
Expand Down Expand Up @@ -130,6 +131,9 @@ func add(mgr manager.Manager, r *Reconciler) error {
},
}}
}))
if err != nil {
return err
}
}

// Watch for changes to ExpansionTemplates
Expand Down Expand Up @@ -193,7 +197,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
}

func (r *Reconciler) queueConflicts(old expansion.IDSet) {
for tID, _ := range symmetricDiff(old, r.system.GetConflicts()) {
for tID := range symmetricDiff(old, r.system.GetConflicts()) {
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(expansionv1alpha1.GroupVersion.WithKind("ExpansionTemplate"))
u.SetName(string(tID))
Expand All @@ -206,12 +210,12 @@ func (r *Reconciler) queueConflicts(old expansion.IDSet) {
func symmetricDiff(x, y expansion.IDSet) expansion.IDSet {
sDiff := make(expansion.IDSet)

for id, _ := range x {
for id := range x {
if _, exists := y[id]; !exists {
sDiff[id] = true
}
}
for id, _ := range y {
for id := range y {
if _, exists := x[id]; !exists {
sDiff[id] = true
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/expansion/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (d *db) handleAdd(template *expansionunversioned.ExpansionTemplate) (bool,
if _, err := d.graph.Vertex(hashID(id)); err != nil {
if errors.Is(err, graph.ErrVertexNotFound) {
if err := d.graph.AddVertex(id); err != nil {
return false, fmt.Errorf("adding vertex to graph: %v", err)
return false, fmt.Errorf("adding vertex to graph: %w", err)
}
} else {
return false, fmt.Errorf("unexpected error getting vertex for template %s: %w", id, err)
Expand All @@ -118,7 +118,7 @@ func (d *db) handleAdd(template *expansionunversioned.ExpansionTemplate) (bool,
}
cycle = createsCycle || cycle

if err := d.graph.AddEdge(from, to); err != nil {
if err = d.graph.AddEdge(from, to); err != nil {
return false, fmt.Errorf("adding edge for template %s: %w", id, err)
}
}
Expand All @@ -133,14 +133,14 @@ func (d *db) edgesForTemplate(template *expansionunversioned.ExpansionTemplate)

// Add out-bound edges (from this template's generated GVK to other
// templates' matched GVKs)
for t, _ := range d.matchers[genGVK] {
for t := range d.matchers[genGVK] {
edges = append(edges, edge{id, t})
}

// Add in-bound edges (from other templates' generated GVK to this template's
// matched GVK)
for _, gen := range applyToGVKs(template) {
for t, _ := range d.generators[gen] {
for t := range d.generators[gen] {
edges = append(edges, edge{t, id})
}
}
Expand Down Expand Up @@ -189,7 +189,7 @@ func (d *db) handleRemove(template *expansionunversioned.ExpansionTemplate) {
from := hashID(e.x)
to := hashID(e.y)
if err := d.graph.RemoveEdge(from, to); err != nil {
panic(fmt.Errorf("[template %q] unexpected error removing edge: %v", id, err))
panic(fmt.Errorf("[template %q] unexpected error removing edge: %w", id, err))
}
}
}
Expand All @@ -202,7 +202,7 @@ func (d *db) updateCycles() {

conflicts, err := graph.StronglyConnectedComponents(d.graph)
if err != nil {
panic(fmt.Errorf("error getting SCCs: %v", err))
panic(fmt.Errorf("error getting SCCs: %w", err))
}
// All strongly connect components containing more than 1 vertex are a cycle
for _, scc := range conflicts {
Expand All @@ -224,7 +224,7 @@ func (d *db) upsert(template *expansionunversioned.ExpansionTemplate) error {

newCycle, err := d.handleAdd(template)
if err != nil {
return fmt.Errorf("adding template to db: %v", err)
return fmt.Errorf("adding template to db: %w", err)
}
// If the new/updated template caused a cycle, or the previous template belonged
// to a cycle, then we need to re-check the graph for cycles
Expand Down Expand Up @@ -255,7 +255,7 @@ func (d *db) remove(template *expansionunversioned.ExpansionTemplate) {

func (d *db) templatesForGVK(gvk schema.GroupVersionKind) []*expansionunversioned.ExpansionTemplate {
var tset []*expansionunversioned.ExpansionTemplate
for tID, _ := range d.matchers[gvk] {
for tID := range d.matchers[gvk] {
// Sanity check. In theory, this should never happen, but if it does, we
// can't recover.
if _, exists := d.store[tID]; !exists {
Expand Down
11 changes: 5 additions & 6 deletions pkg/expansion/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func TestDB(t *testing.T) {
{
name: "update template to produce cycle",
ops: []templateOperation{
//t5 is completely disjoint from rest of graph
// t5 is completely disjoint from rest of graph
{
op: addOp,
template: *fixtures.TestTemplate("t5", 5, 6),
Expand Down Expand Up @@ -520,8 +520,6 @@ func TestDB(t *testing.T) {
schema.GroupVersionKind{Group: "group3", Version: "v3", Kind: "kind3"}: {
keyForTemplate(fixtures.TestTemplate("t2-3", 2, 3)): true,
},
// keyForTemplate(fixtures.TestTemplate("t3-1", 3, 4)): true,
//},
schema.GroupVersionKind{Group: "group8", Version: "v8", Kind: "kind8"}: {
keyForTemplate(fixtures.TestTemplate("t7-8", 7, 8)): true,
},
Expand Down Expand Up @@ -621,7 +619,8 @@ func TestDB(t *testing.T) {
}

func executeOps(db templateDB, ops []templateOperation, t *testing.T) {
for _, op := range ops {
for i := 0; i < len(ops); i++ {
op := ops[i]
switch op.op {
case addOp:
gotErr := db.upsert(&op.template)
Expand Down Expand Up @@ -783,14 +782,14 @@ func TestGetConflicts(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
d := newDB()
for i, _ := range tc.seed {
for i := range tc.seed {
d.store[keyForTemplate(tc.seed[i].template)] = &tc.seed[i]
}

got := d.getConflicts()

require.Len(t, got, len(tc.want))
for k, _ := range tc.want {
for k := range tc.want {
if _, exists := got[k]; !exists {
t.Errorf("wanted template ID %s, but not returned", k)
}
Expand Down
14 changes: 1 addition & 13 deletions pkg/expansion/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ func TestValidateTemplate(t *testing.T) {
{
name: "missing name",
temp: *fixtures.NewTemplate(&fixtures.TemplateData{
//Name: "test1",
Apply: []match.ApplyTo{{
Groups: []string{"apps"},
Kinds: []string{"Deployment"},
Expand All @@ -331,7 +330,6 @@ func TestValidateTemplate(t *testing.T) {
Kinds: []string{"Deployment"},
Versions: []string{"v1"},
}},
//Source: "spec.template",
GenGVK: expansionunversioned.GeneratedGVK{
Group: "",
Version: "v1",
Expand All @@ -350,23 +348,13 @@ func TestValidateTemplate(t *testing.T) {
Versions: []string{"v1"},
}},
Source: "spec.template",
//GenGVK: expansionunversioned.GeneratedGVK{
// Group: "",
// Version: "v1",
// Kind: "Pod",
//},
}),
errFn: matchErr("empty generatedGVK"),
},
{
name: "missing applyTo",
temp: *fixtures.NewTemplate(&fixtures.TemplateData{
Name: "test1",
//Apply: []match.ApplyTo{{
// Groups: []string{"apps"},
// Kinds: []string{"Deployment"},
// Versions: []string{"v1"},
//}},
Name: "test1",
Source: "spec.template",
GenGVK: expansionunversioned.GeneratedGVK{
Group: "",
Expand Down

0 comments on commit 2357e4a

Please sign in to comment.