Skip to content

Commit

Permalink
Static Check Resolutions (#192)
Browse files Browse the repository at this point in the history
* Static check resolutions

* Corrected misspellings throughout code

* adding staticcheck to make check-formatting and including it in deps for install

* removing dead tests
  • Loading branch information
kensipe committed Apr 16, 2019
1 parent 1ed7170 commit 55ae38d
Show file tree
Hide file tree
Showing 24 changed files with 79 additions and 258 deletions.
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ deps:
dep check -skip-vendor
go install github.com/kudobuilder/kudo/vendor/golang.org/x/tools/cmd/goimports
go install github.com/kudobuilder/kudo/vendor/golang.org/x/lint/golint
go get -u honnef.co/go/tools/cmd/staticcheck

# Run tests
test: generate deps fmt vet lint imports manifests
Expand All @@ -21,7 +22,7 @@ test: generate deps fmt vet lint imports manifests
test-clean:
rm -f cover.out

check-formatting: generate deps vet lint
check-formatting: generate deps vet lint staticcheck
./test/formatting.sh

prebuild: generate fmt vet
Expand Down
8 changes: 4 additions & 4 deletions config/crds/kudo_v1alpha1_framework.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ spec:
validation:
openAPIV3Schema:
properties:
apiVersion:
type: string
kind:
type: string
inline:
type: object
metadata:
type: object
spec:
type: object
status:
type: object
required:
- inline
version: v1alpha1
status:
acceptedNames:
Expand Down
8 changes: 4 additions & 4 deletions config/crds/kudo_v1alpha1_frameworkversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ spec:
validation:
openAPIV3Schema:
properties:
apiVersion:
type: string
kind:
type: string
inline:
type: object
metadata:
type: object
spec:
Expand Down Expand Up @@ -70,6 +68,8 @@ spec:
type: object
status:
type: object
required:
- inline
version: v1alpha1
status:
acceptedNames:
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kudo/v1alpha1/framework_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type FrameworkStatus struct {
// Framework is the Schema for the frameworks API
// +k8s:openapi-gen=true
type Framework struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec FrameworkSpec `json:"spec,omitempty"`
Expand All @@ -51,7 +51,7 @@ type Framework struct {

// FrameworkList contains a list of Framework
type FrameworkList struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []Framework `json:"items"`
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kudo/v1alpha1/frameworkversion_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ type FrameworkVersionStatus struct {
// FrameworkVersion is the Schema for the frameworkversions API
// +k8s:openapi-gen=true
type FrameworkVersion struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec FrameworkVersionSpec `json:"spec,omitempty"`
Expand All @@ -144,7 +144,7 @@ type FrameworkVersion struct {

// FrameworkVersionList contains a list of FrameworkVersion
type FrameworkVersionList struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []FrameworkVersion `json:"items"`
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/apis/kudo/v1alpha1/frameworkversion_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,12 @@ func TestFrameworkType_Validation(t *testing.T) {
receivedErrorList := createSlice(vErr)

diff := compareSlice(receivedErrorList, tt.err)
if diff != nil {
for _, err := range diff {
t.Errorf("Found unexpected error: %v", err)
}
for _, err := range diff {
t.Errorf("Found unexpected error: %v", err)
}
missing := compareSlice(tt.err, receivedErrorList)
if missing != nil {
for _, err := range missing {
t.Errorf("Missed expected error: %v", err)
}
for _, err := range missing {
t.Errorf("Missed expected error: %v", err)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kudo/v1alpha1/instance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const PhaseStateSuspend PhaseState = "SUSPEND"
// Instance is the Schema for the instances API
// +k8s:openapi-gen=true
type Instance struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec InstanceSpec `json:"spec,omitempty"`
Expand All @@ -98,7 +98,7 @@ type Instance struct {

// InstanceList contains a list of Instance
type InstanceList struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []Instance `json:"items"`
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kudo/v1alpha1/planexecution_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type StepStatus struct {
// PlanExecution is the Schema for the planexecutions API
// +k8s:openapi-gen=true
type PlanExecution struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec PlanExecutionSpec `json:"spec,omitempty"`
Expand All @@ -81,7 +81,7 @@ type PlanExecution struct {

// PlanExecutionList contains a list of PlanExecution
type PlanExecutionList struct {
metav1.TypeMeta `json:",inline"`
metav1.TypeMeta `json:"inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []PlanExecution `json:"items"`
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan
func StartTestManager(mgr manager.Manager, g *gomega.GomegaWithT) (chan struct{}, *sync.WaitGroup) {
stop := make(chan struct{})
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
wg.Add(1)
g.Expect(mgr.Start(stop)).NotTo(gomega.HaveOccurred())
wg.Done()
}()
Expand Down
36 changes: 0 additions & 36 deletions pkg/controller/framework/framework_controller_test.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan
func StartTestManager(mgr manager.Manager, g *gomega.GomegaWithT) (chan struct{}, *sync.WaitGroup) {
stop := make(chan struct{})
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
wg.Add(1)
g.Expect(mgr.Start(stop)).NotTo(gomega.HaveOccurred())
wg.Done()
}()
Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions pkg/controller/instance/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"
)

const basePath = "/kustomize"

/**
* USER ACTION REQUIRED: This is a scaffold file intended for the user to modify with their own Controller
* business logic. Delete these comments after modifying this file.*
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/instance/instance_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan
func StartTestManager(mgr manager.Manager, g *gomega.GomegaWithT) (chan struct{}, *sync.WaitGroup) {
stop := make(chan struct{})
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
wg.Add(1)
g.Expect(mgr.Start(stop)).NotTo(gomega.HaveOccurred())
wg.Done()
}()
Expand Down
36 changes: 0 additions & 36 deletions pkg/controller/instance/instance_controller_test.go

This file was deleted.

28 changes: 14 additions & 14 deletions pkg/controller/planexecution/planexecution_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
//Watch for Deployments, Jobs and StatefulSets
// Define a mapping from the object in the event to one or more
// objects to Reconcile. Specifically this calls for
// a reconsiliation of any objects "Owner".
// a reconciliation of any objects "Owner".
mapFn := handler.ToRequestsFunc(
func(a handler.MapObject) []reconcile.Request {
owners := a.Meta.GetOwnerReferences()
Expand Down Expand Up @@ -263,7 +263,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
//Before returning from this function, update the status
defer r.Update(context.Background(), planExecution)

//need to add ownerRefernce as the Instance
//need to add ownerReference as the Instance

instance.Status.ActivePlan = corev1.ObjectReference{
Name: planExecution.Name,
Expand Down Expand Up @@ -298,7 +298,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
}

//Load parameters:
//Create configmap to hold all parameters for instantiation
//Create config map to hold all parameters for instantiation
configs := make(map[string]string)
//Default parameters from instance metadata
configs["FRAMEWORK_NAME"] = frameworkVersion.Spec.Framework.Name
Expand All @@ -313,7 +313,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
_, ok := configs[param.Name]
if !ok { //not specified in params
if param.Required {
err = fmt.Errorf("Parameter %v was required but not provided by instance %v", param.Name, instance.Name)
err = fmt.Errorf("parameter %v was required but not provided by instance %v", param.Name, instance.Name)
log.Printf("PlanExecutionController: %v", err)
r.recorder.Event(planExecution, "Warning", "MissingParameter", fmt.Sprintf("Could not find required parameter (%v)", param.Name))
return reconcile.Result{}, err
Expand All @@ -331,7 +331,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
executedPlan, ok := frameworkVersion.Spec.Plans[planExecution.Spec.PlanName]
if !ok {
r.recorder.Event(planExecution, "Warning", "InvalidPlan", fmt.Sprintf("Could not find required plan (%v)", planExecution.Spec.PlanName))
err = fmt.Errorf("Could not find required plan (%v)", planExecution.Spec.PlanName)
err = fmt.Errorf("could not find required plan (%v)", planExecution.Spec.PlanName)
planExecution.Status.State = kudov1alpha1.PhaseStateError
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -499,8 +499,8 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
return fmt.Errorf("object passed in doesn't match expected StatefulSet type")
}

// We need some specilized logic in there. We can't jsut copy the Spec since there are other values
// like spec.updateStrate, spec.volumeClaimTemplates, etc that are all
// We need some specialized logic in there. We can't just copy the Spec since there are other values
// like spec.updateState, spec.volumeClaimTemplates, etc that are all
// generated from the object by the k8s controller. We just want to update things we can change
newSs.Spec.Replicas = ss.Spec.Replicas

Expand Down Expand Up @@ -621,14 +621,14 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
return reconcile.Result{}, nil
}

//Cleanup modfies objects on the cluster to allow for the provided obj to get CreateOrApply. Currently
//Cleanup modifies objects on the cluster to allow for the provided obj to get CreateOrApply. Currently
//only needs to clean up Jobs that get run from multiple PlanExecutions
func (r *ReconcilePlanExecution) Cleanup(obj runtime.Object) error {
switch obj.(type) {

switch obj := obj.(type) {
case *batchv1.Job:
//We need to see if there's a current job on the system that matches this exactly (with labels)
job := obj.(*batchv1.Job)
log.Printf("PlanExecutionController.Cleanup: *batchv1.Job %v", job.Name)
log.Printf("PlanExecutionController.Cleanup: *batchv1.Job %v", obj.Name)

present := &batchv1.Job{}
key, _ := client.ObjectKeyFromObject(obj)
Expand All @@ -643,7 +643,7 @@ func (r *ReconcilePlanExecution) Cleanup(obj runtime.Object) error {
return err
}
//see if the job in the cluster has the same labels as the one we're looking to add.
for k, v := range job.Labels {
for k, v := range obj.Labels {
if v != present.Labels[k] {
//need to delete the present job since its got labels that aren't the same
log.Printf("PlanExecutionController: Different values for job key \"%v\": \"%v\" and \"%v\"", k, v, present.Labels[k])
Expand All @@ -652,9 +652,9 @@ func (r *ReconcilePlanExecution) Cleanup(obj runtime.Object) error {
}
}
for k, v := range present.Labels {
if v != job.Labels[k] {
if v != obj.Labels[k] {
//need to delete the present job since its got labels that aren't the same
log.Printf("PlanExecutionController: Different values for job key \"%v\": \"%v\" and \"%v\"", k, v, job.Labels[k])
log.Printf("PlanExecutionController: Different values for job key \"%v\": \"%v\" and \"%v\"", k, v, obj.Labels[k])
err = r.Delete(context.TODO(), present)
return err
}
Expand Down
Loading

0 comments on commit 55ae38d

Please sign in to comment.