Skip to content
Permalink
Browse files

Adds golintCI to travis job (#2332)

* What kind of PR is this?

/kind cleanup

What does does this PR do / why we need it:

It adds golangci-lint as a travis job
It also adds the golangci-lint as make target.
It also fixes the current lint errors in our code base.

Which issue(s) this PR fixes:

Fixes #1927

How to test changes / Special notes to the reviewer:

run `golangci-lint run` and check that no errors occur

Signed-off-by: mik-dass <mrinald7@gmail.com>

* Fixes watch test by adding timeout
  • Loading branch information...
mik-dass authored and openshift-merge-robot committed Nov 7, 2019
1 parent 18bbde5 commit c281978f9c467d3baf3632520058ee5ff0762cef
@@ -17,11 +17,14 @@ jobs:
go: "1.11.2"
install:
- make goget-tools
# binary will be $(go env GOPATH)/bin/golangci-lint
- curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.21.0
script:
- export PATH="$PATH:$GOPATH/bin"
- make bin
- make validate
- make test-coverage
- make golint
after_success:
# submit coverage.txt to codecov.io
- bash <(curl -s https://codecov.io/bash)
@@ -63,6 +63,11 @@ check-vendor:
.PHONY: validate-vendor-licenses
validate-vendor-licenses:
wwhrd check -q

.PHONY: golint
golint:
golangci-lint run ./... --timeout 5m

# golint errors are only recommendations
.PHONY: lint
lint:
@@ -13,11 +13,9 @@ import (
)

const (
appPrefixMaxLen = 12
appNameMaxRetries = 3
appAPIVersion = "odo.openshift.io/v1alpha1"
appKind = "Application"
appList = "List"
appAPIVersion = "odo.openshift.io/v1alpha1"
appKind = "Application"
appList = "List"
)

// List all applications in current project
@@ -99,7 +99,7 @@ func Login(server, username, password, token, caAuth string, skipTLS bool) error
// Kindly taken from https://stackoverflow.com/questions/54570268/filtering-the-output-of-a-terminal-output-using-golang
func copyAndFilter(w io.Writer, r io.Reader) ([]byte, error) {
var out []byte
buf := make([]byte, 1024, 1024)
buf := make([]byte, 1024)
for {
n, err := r.Read(buf[:])
if n > 0 {
@@ -899,11 +899,11 @@ func TestGetComponentFromConfig(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg, err := NewLocalConfigInfo("")
_, err := NewLocalConfigInfo("")
if err != nil {
t.Error(err)
}
cfg = &tt.existingConfig
cfg := &tt.existingConfig

got, _ := GetComponentFromConfig(*cfg)

@@ -98,6 +98,9 @@ func addRecursiveWatch(watcher *fsnotify.Watcher, path string, ignores []string)
}
return nil
})
if err != nil {
return err
}
for _, folder := range folders {

if matched, _ := util.IsGlobExpMatch(folder, ignores); matched {
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"testing"
"time"

"github.com/openshift/odo/pkg/occlient"
"github.com/openshift/odo/pkg/testingutil"
@@ -80,23 +81,18 @@ var StartChan = make(chan bool)
// Mock PushLocal to collect changed files and compare against expected changed files
func mockPushLocal(client *occlient.Client, componentName string, applicationName string, path string, out io.Writer, files []string, delFiles []string, isPushForce bool, globExps []string, show bool) error {
for _, gotChangedFile := range files {
found := false
// Verify every file in expected file changes to be actually observed to be changed
// If found exactly same or different, return from PushLocal and signal exit for watch so that the watch terminates gracefully
for _, expChangedFile := range ExpectedChangedFiles {
wantedFileDetail := CompDirStructure[expChangedFile]
if filepath.Join(wantedFileDetail.FileParent, wantedFileDetail.FilePath) == gotChangedFile {
found = true
ExtChan <- true
return nil
}
}
if !found {
ExtChan <- true
return fmt.Errorf("received %+v which is not same as expected list %+v", files, ExpectedChangedFiles)
}
}
return nil
ExtChan <- true
return fmt.Errorf("received %+v which is not same as expected list %+v", files, ExpectedChangedFiles)
}

func TestWatchAndPush(t *testing.T) {
@@ -233,6 +229,7 @@ func TestWatchAndPush(t *testing.T) {
go func() {
t.Logf("Starting file simulations \n%+v\n", tt.fileModifications)
// Simulating file modifications for watch to observe
pingTimeout := time.After(time.Duration(1) * time.Minute)
for {
select {
case startMsg := <-StartChan:
@@ -266,7 +263,8 @@ func TestWatchAndPush(t *testing.T) {
}
}
t.Logf("The CompDirStructure is \n%+v\n", CompDirStructure)
return
case <-pingTimeout:
break
}
}
}()
@@ -664,7 +664,7 @@ func (lci *LocalConfigInfo) GetOSSourcePath() (path string, err error) {

// Get the component context folder
// ".odo" is removed as lci.Filename will always return the '.odo' folder.. we don't need that!
componentContext := strings.Trim(filepath.Dir(lci.Filename), ".odo")
componentContext := strings.TrimSuffix(filepath.Dir(lci.Filename), ".odo")

if sourceLocation == "" {
return "", fmt.Errorf("Blank source location, does the .odo directory exist?")
@@ -688,5 +688,5 @@ func (lci *LocalConfigInfo) GetOSSourcePath() (path string, err error) {

sourceOSPath := filepath.FromSlash(absPath)

return sourceOSPath, nil
return sourceOSPath, err
}
@@ -393,20 +393,6 @@ func (c *Client) GetPortsFromBuilderImage(componentType string) ([]string, error
return portList, nil
}

// isLoggedIn checks whether user is logged in or not and returns boolean output
func (c *Client) isLoggedIn() bool {
// ~ indicates current user
// Reference: https://github.com/openshift/origin/blob/master/pkg/oc/cli/cmd/whoami.go#L55
output, err := c.userClient.Users().Get("~", metav1.GetOptions{})
glog.V(4).Infof("isLoggedIn err: %#v \n output: %#v", err, output.Name)
if err != nil {
glog.V(4).Info(errors.Wrap(err, "error running command"))
glog.V(4).Infof("Output is: %v", output)
return false
}
return true
}

// RunLogout logs out the current user from cluster
func (c *Client) RunLogout(stdout io.Writer) error {
output, err := c.userClient.Users().Get("~", metav1.GetOptions{})
@@ -597,7 +583,10 @@ func addLabelsToArgs(labels map[string]string, args []string) []string {
// getExposedPortsFromISI parse ImageStreamImage definition and return all exposed ports in form of ContainerPorts structs
func getExposedPortsFromISI(image *imagev1.ImageStreamImage) ([]corev1.ContainerPort, error) {
// file DockerImageMetadata
imageWithMetadata(&image.Image)
err := imageWithMetadata(&image.Image)
if err != nil {
return nil, err
}

var ports []corev1.ContainerPort

@@ -755,12 +744,9 @@ func (c *Client) GetImageStreamImage(imageStream *imagev1.ImageStream, imageTag
imageNS := imageStream.ObjectMeta.Namespace
imageName := imageStream.ObjectMeta.Name

tagFound := false

for _, tag := range imageStream.Status.Tags {
// look for matching tag
if tag.Tag == imageTag {
tagFound = true
glog.V(4).Infof("Found exact image tag match for %s:%s", imageName, imageTag)

if len(tag.Items) > 0 {
@@ -780,12 +766,8 @@ func (c *Client) GetImageStreamImage(imageStream *imagev1.ImageStream, imageTag
}
}

if !tagFound {
return nil, fmt.Errorf("unable to find tag %s for image %s", imageTag, imageName)
}

// return error since its an unhandled case if code reaches here
return nil, fmt.Errorf("unable to fetch image with tag %s corresponding to imagestream %+v", imageTag, imageStream)
return nil, fmt.Errorf("unable to find tag %s for image %s", imageTag, imageName)
}

// GetImageStreamTags returns all the ImageStreamTag objects in the given namespace
@@ -851,7 +833,6 @@ func (c *Client) NewAppS2I(params CreateArgs, commonObjectMeta metav1.ObjectMeta
if err != nil {
return errors.Wrapf(err, "unable to create s2i app for %s", commonObjectMeta.Name)
}
imageNS = imageStream.ObjectMeta.Namespace
containerPorts, err = util.GetContainerPortsFromStrings(params.Ports)
if err != nil {
return errors.Wrapf(err, "unable to get container ports from %v", params.Ports)
@@ -1288,9 +1269,7 @@ func (c *Client) UpdateBuildConfig(buildConfigName string, gitURL string, annota
}

// generate BuildConfig
buildSource := buildv1.BuildSource{}

buildSource = buildv1.BuildSource{
buildSource := buildv1.BuildSource{
Git: &buildv1.GitBuildSource{
URI: gitURL,
},
@@ -1381,9 +1360,9 @@ func (c *Client) PatchCurrentDC(dc appsv1.DeploymentConfig, prePatchDCHandler dc
if reflect.DeepEqual(updatedDc.Spec.Template, currentDC.Spec.Template) {
return nil
} else {
currentDCBytes, err := json.Marshal(currentDC.Spec.Template)
updatedDCBytes, err := json.Marshal(updatedDc.Spec.Template)
if err != nil {
currentDCBytes, errCurrent := json.Marshal(currentDC.Spec.Template)
updatedDCBytes, errUpdated := json.Marshal(updatedDc.Spec.Template)
if errCurrent != nil || errUpdated != nil {
return errors.Wrapf(err, "unable to unmarshal dc")
}
glog.V(4).Infof("going to wait for new deployment roll out because updatedDc Spec.Template: %v doesn't match currentDc Spec.Template: %v", string(updatedDCBytes), string(currentDCBytes))
@@ -1471,9 +1450,7 @@ func (c *Client) UpdateDCToGit(ucp UpdateComponentParams, isDeleteSupervisordVol
return errors.New("UpdateDCToGit imageName cannot be blank")
}

var dc appsv1.DeploymentConfig

dc = generateGitDeploymentConfig(ucp.CommonObjectMeta, ucp.ImageMeta.Name, ucp.ImageMeta.Ports, ucp.EnvVars, &ucp.ResourceLimits)
dc := generateGitDeploymentConfig(ucp.CommonObjectMeta, ucp.ImageMeta.Name, ucp.ImageMeta.Ports, ucp.EnvVars, &ucp.ResourceLimits)

if isDeleteSupervisordVolumes {
// Patch the current DC
@@ -2407,23 +2384,6 @@ func (c *Client) GetAllClusterServicePlans() ([]scv1beta1.ClusterServicePlan, er
return planList.Items, nil
}

// imageStreamExists returns true if the given image stream exists in the given
// namespace
func (c *Client) imageStreamExists(name string, namespace string) bool {
imageStreams, err := c.GetImageStreamsNames(namespace)
if err != nil {
glog.V(4).Infof("unable to get image streams in the namespace: %v", namespace)
return false
}

for _, is := range imageStreams {
if is == name {
return true
}
}
return false
}

// CreateRoute creates a route object for the given service and with the given labels
// serviceName is the name of the service for the target reference
// portNumber is the target port of the route
@@ -2690,8 +2650,7 @@ func (c *Client) CopyFile(localPath string, targetPodName string, targetPath str
go func() {
defer writer.Close()

var err error
err = makeTar(localPath, dest, writer, copyFiles, globExps)
err := makeTar(localPath, dest, writer, copyFiles, globExps)
if err != nil {
glog.Errorf("Error while creating tar: %#v", err)
os.Exit(1)
@@ -2717,10 +2676,7 @@ func (c *Client) CopyFile(localPath string, targetPodName string, targetPath str
// checkFileExist check if given file exists or not
func checkFileExist(fileName string) bool {
_, err := os.Stat(fileName)
if os.IsNotExist(err) {
return false
}
return true
return !os.IsNotExist(err)
}

// makeTar function is copied from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/cp.go#L309
@@ -2762,43 +2718,6 @@ func makeTar(srcPath, destPath string, writer io.Writer, files []string, globExp
return nil
}

// Tar will be used to tar files using odo watch
// inspired from https://gist.github.com/jonmorehouse/9060515
func tar(tw *taro.Writer, fileName string, destFile string) error {
stat, _ := os.Lstat(fileName)

// now lets create the header as needed for this file within the tarball
hdr, err := taro.FileInfoHeader(stat, fileName)
if err != nil {
return err
}
splitFileName := strings.Split(fileName, destFile)[1]

// hdr.Name can have only '/' as path separator, next line makes sure there is no '\'
// in hdr.Name on Windows by replacing '\' to '/' in splitFileName. destFile is
// a result of path.Base() call and never have '\' in it.
hdr.Name = destFile + strings.Replace(splitFileName, "\\", "/", -1)
// write the header to the tarball archive
err = tw.WriteHeader(hdr)
if err != nil {
return err
}

file, err := os.Open(fileName)
if err != nil {
return err
}
defer file.Close()

// copy the file data to the tarball
_, err = io.Copy(tw, file)
if err != nil {
return err
}

return nil
}

// recursiveTar function is copied from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/cp.go#L319
func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *taro.Writer, globExps []string) error {
glog.V(4).Infof("recursiveTar arguments: srcBase: %s, srcFile: %s, destBase: %s, destFile: %s", srcBase, srcFile, destBase, destFile)
@@ -23,7 +23,6 @@ import (
componentlabels "github.com/openshift/odo/pkg/component/labels"
"github.com/openshift/odo/pkg/config"
"github.com/openshift/odo/pkg/testingutil"
"github.com/openshift/odo/pkg/util"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -135,23 +134,6 @@ func fakeResourceRequirements() *corev1.ResourceRequirements {
return &resReq
}

func fakeResourceConsumption() []util.ResourceRequirementInfo {
memoryQuantity, err := util.FetchResourceQuantity(corev1.ResourceMemory, "100Mi", "350Mi", "")
if err != nil {
fmt.Println(err)
return nil
}
cpuQuantity, err := util.FetchResourceQuantity(corev1.ResourceCPU, "100m", "350m", "")
if err != nil {
fmt.Println(err)
return nil
}
return []util.ResourceRequirementInfo{
*memoryQuantity,
*cpuQuantity,
}
}

// fakeImageStream gets imagestream for the reactor
func fakeImageStream(imageName string, namespace string, strTags []string) *imagev1.ImageStream {
var tags []imagev1.NamedTagEventList
@@ -91,10 +91,7 @@ func (c *Client) DeletePVC(name string) error {

// IsAppSupervisorDVolume checks if the volume is a supervisorD volume
func (c *Client) IsAppSupervisorDVolume(volumeName, dcName string) bool {
if volumeName == getAppRootVolumeName(dcName) {
return true
}
return false
return volumeName == getAppRootVolumeName(dcName)
}

// getVolumeNamesFromPVC returns the name of the volume associated with the given

0 comments on commit c281978

Please sign in to comment.
You can’t perform that action at this time.