Skip to content

Commit

Permalink
Various simplifications, bug fixes and removals of deprecated APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
bvwells committed Oct 5, 2020
1 parent 9b61c96 commit 04deac2
Show file tree
Hide file tree
Showing 59 changed files with 181 additions and 137 deletions.
9 changes: 2 additions & 7 deletions config/tests/jobs/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,11 +753,6 @@ func checkScenarioArgs(jobName, imageName string, args []string) error {
}

if scenario == "" {
entry := jobName
if strings.HasPrefix(jobName, "pull-security-kubernetes") {
entry = strings.Replace(entry, "pull-security-kubernetes", "pull-kubernetes", -1)
}

if !scenarioArgs {
if strings.Contains(imageName, "kubekins-e2e") ||
strings.Contains(imageName, "bootstrap") ||
Expand Down Expand Up @@ -1115,8 +1110,8 @@ func TestK8sInfraProwBuildJobsMustNotExceedTotalCapacity(t *testing.T) {
}
}
for _, r := range resourceNames {
total, _ := totalLimit[r]
max, _ := maxLimit[r]
total := totalLimit[r]
max := maxLimit[r]
if total.Cmp(max) > -1 {
t.Errorf("Total %s limit %s greater than expected limit %s", r, total.String(), max.String())
}
Expand Down
4 changes: 0 additions & 4 deletions config/tests/testgrids/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,18 +486,14 @@ func TestKubernetesProwInstanceJobsMustHaveMatchingTestgridEntries(t *testing.T)
}

// Conclusion
badjobs := []string{}
for job, valid := range jobs {
if !valid {
badjobs = append(badjobs, job)
t.Errorf("Job %v does not have a matching testgrid testgroup", job)
}
}

badconfigs := []string{}
for testgroup, valid := range testgroups {
if !valid {
badconfigs = append(badconfigs, testgroup)
t.Errorf("Testgrid group %v is supposed to be moved to have their presubmits in testgrid. See this issue: https://github.com/kubernetes/test-infra/issues/18159", testgroup)
}
}
Expand Down
10 changes: 5 additions & 5 deletions experiment/coverage/apicoverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func parseOpenAPI(rawdata []byte) apiArray {
log.Fatal(err)
}
t, ok := methodSpec.(*spec.Operation)
if ok == false {
if !ok {
log.Fatal("Failed to convert methodSpec.")
}
if t == nil {
Expand Down Expand Up @@ -205,14 +205,14 @@ func getTestedAPIsByLevel(negative bool, reg *regexp.Regexp, apisOpenapi, apisTe
var apisAllByLevel apiArray

for _, openapi := range apisTested {
if (negative == false && reg.MatchString(openapi.URL)) ||
(negative == true && !reg.MatchString(openapi.URL)) {
if (!negative && reg.MatchString(openapi.URL)) ||
(negative && !reg.MatchString(openapi.URL)) {
apisTestedByLevel = append(apisTestedByLevel, openapi)
}
}
for _, openapi := range apisOpenapi {
if (negative == false && reg.MatchString(openapi.URL)) ||
(negative == true && !reg.MatchString(openapi.URL)) {
if (!negative && reg.MatchString(openapi.URL)) ||
(negative && !reg.MatchString(openapi.URL)) {
apisAllByLevel = append(apisAllByLevel, openapi)
}
}
Expand Down
1 change: 1 addition & 0 deletions experiment/slack-oncall-updater/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func getJSON(url string, v interface{}) error {
if err != nil {
return fmt.Errorf("failed to fetch %q: %v", url, err)
}
defer resp.Body.Close()
decoder := json.NewDecoder(resp.Body)
err = decoder.Decode(v)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions gcsweb/cmd/gcsweb/gcsweb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (s *gcsMockServer) getObjectRaw(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Length", strconv.Itoa(len(obj.Content)))
w.Header().Set("Last-Modified", obj.Updated.Format(http.TimeFormat))
w.WriteHeader(http.StatusOK)
fmt.Fprintf(w, string(obj.Content))
fmt.Fprint(w, string(obj.Content))
return
}
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func TestHandleObject(t *testing.T) {
if err == nil && tc.errorExpected {
t.Fatalf("Error was expected")
}
actualHeaders := w.HeaderMap
actualHeaders := w.Result().Header
actualBody := w.Body.String()

if !reflect.DeepEqual(actualHeaders, tc.expectedHeaders) {
Expand Down Expand Up @@ -426,7 +426,7 @@ func TestHandleDirectory(t *testing.T) {
t.Fatalf("error not expected: %v", err)
}

actualBody := fmt.Sprintf("%s", w.Body.String())
actualBody := w.Body.String()
if !reflect.DeepEqual(actualBody, tc.expected) {
t.Fatal(cmp.Diff(actualBody, tc.expected))
}
Expand Down
2 changes: 1 addition & 1 deletion greenhouse/eviction.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func monitorDiskAndEvict(
logger.WithError(err).Errorf("Error deleting entry at path: %v", entry.Path)
} else {
promMetrics.FilesEvicted.Inc()
promMetrics.LastEvictedAccessAge.Set(time.Now().Sub(entry.LastAccess).Hours())
promMetrics.LastEvictedAccessAge.Set(time.Since(entry.LastAccess).Hours())
}
// get new disk usage
blocksFree, _, _, err = diskutil.GetDiskUsage(diskRoot)
Expand Down
3 changes: 1 addition & 2 deletions kubetest/aks.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ func newSSHKeypair(bits int) (private, public []byte, err error) {
}

func (a *aksDeployer) dockerLogin() error {
cmd := &exec.Cmd{}
username := ""
pwd := ""
server := ""
Expand All @@ -368,7 +367,7 @@ func (a *aksDeployer) dockerLogin() error {
pwd = a.azureCreds.ClientSecret
server = imageRegistry
}
cmd = exec.Command("docker", "login", fmt.Sprintf("--username=%s", username), fmt.Sprintf("--password=%s", pwd), server)
cmd := exec.Command("docker", "login", fmt.Sprintf("--username=%s", username), fmt.Sprintf("--password=%s", pwd), server)
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("failed Docker login with output %s\n error: %v", out, err)
}
Expand Down
12 changes: 9 additions & 3 deletions kubetest/aksengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,11 @@ func (c *aksEngineDeployer) SetCustomCloudProfileEnvironment() error {
Timeout: 30 * time.Second,
}
endpointsresp, err := httpClient.Get(metadataURL)
if err != nil || endpointsresp.StatusCode != 200 {
if err != nil {
return fmt.Errorf("%s . apimodel invalid: failed to retrieve Azure Stack endpoints from %s", err, metadataURL)
}
defer endpointsresp.Body.Close()
if endpointsresp.StatusCode != 200 {
return fmt.Errorf("%s . apimodel invalid: failed to retrieve Azure Stack endpoints from %s", err, metadataURL)
}

Expand Down Expand Up @@ -696,6 +700,9 @@ func (c *aksEngineDeployer) loadARMTemplates() error {
func (c *aksEngineDeployer) getAzureClient(ctx context.Context) error {
// instantiate Azure Resource Manager Client
env, err := azure.EnvironmentFromName(c.azureEnvironment)
if err != nil {
return err
}
var client *AzureClient
if c.isAzureStackCloud() && strings.EqualFold(c.azureIdentitySystem, ADFSIdentitySystem) {
if client, err = getAzureClient(env,
Expand Down Expand Up @@ -758,7 +765,6 @@ func (c *aksEngineDeployer) createCluster() error {
func (c *aksEngineDeployer) dockerLogin() error {
cwd, _ := os.Getwd()
log.Printf("CWD %v", cwd)
cmd := &exec.Cmd{}
username := ""
pwd := ""
server := ""
Expand All @@ -781,7 +787,7 @@ func (c *aksEngineDeployer) dockerLogin() error {
pwd = c.credentials.ClientSecret
server = imageRegistry
}
cmd = exec.Command("docker", "login", fmt.Sprintf("--username=%s", username), fmt.Sprintf("--password=%s", pwd), server)
cmd := exec.Command("docker", "login", fmt.Sprintf("--username=%s", username), fmt.Sprintf("--password=%s", pwd), server)
if err = cmd.Run(); err != nil {
return fmt.Errorf("failed Docker login with error: %v", err)
}
Expand Down
5 changes: 1 addition & 4 deletions kubetest/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func (s *stageStrategy) Enabled() bool {
// Essentially release/push-build.sh --bucket=B --ci? --gcs-suffix=S --noupdatelatest
func (s *stageStrategy) Stage(noAllowDup bool) error {
name := util.K8s("release", "push-build.sh")
b := s.bucket
if strings.HasPrefix(b, "gs://") {
b = b[len("gs://"):]
}
b := strings.TrimPrefix(s.bucket, "gs://")
args := []string{
"--nomock",
"--verbose",
Expand Down
4 changes: 1 addition & 3 deletions kubetest/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ func AppendError(errs []error, err error) []error {
// Home returns $HOME/part/part/part
func Home(parts ...string) string {
p := []string{os.Getenv("HOME")}
for _, a := range parts {
p = append(p, a)
}
p = append(p, parts...)
return filepath.Join(p...)
}

Expand Down
2 changes: 1 addition & 1 deletion maintenance/migratestatus/migrator/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func retireAction(origContext, newContext, targetURL string) func(statuses []git
stateSuccess := "success"
var desc string
if newContext == "" {
desc = fmt.Sprint("Context retired without replacement.")
desc = "Context retired without replacement."
} else {
desc = fmt.Sprintf("Context retired. Status moved to \"%s\".", newContext)
}
Expand Down
2 changes: 1 addition & 1 deletion prow/clonerefs/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func addHostFingerprints(fingerprints []string) (string, []clone.Command, error)

ssh, err := exec.LookPath("ssh")
cmd = clone.Command{
Command: fmt.Sprintf("golang: lookup ssh path"),
Command: "golang: lookup ssh path",
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions prow/cmd/branchprotector/protect.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (p *protector) protect() {
// know which repos exist. Repos that use in-repo config will appear here,
// because we generate a verification job for them
for repo := range p.cfg.PresubmitsStatic {
if p.completedRepos[repo] == true {
if p.completedRepos[repo] {
continue
}
parts := strings.Split(repo, "/")
Expand Down Expand Up @@ -494,7 +494,7 @@ func equalAdminEnforcement(state github.EnforceAdmins, request *bool) bool {
// bound by the branch protection rules. Therefore, making no
// request is equivalent to making a request to not enforce
// rules on admins.
return state.Enabled == false
return !state.Enabled
default:
return state.Enabled == *request
}
Expand Down
2 changes: 1 addition & 1 deletion prow/cmd/crier/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestOptions(t *testing.T) {
var defaultGitHubOptions flagutil.GitHubOptions
defaultGitHubOptions.AddFlags(flag.NewFlagSet("", flag.ContinueOnError))

defaultGerritProjects := make(map[string][]string, 0)
defaultGerritProjects := make(map[string][]string)

defaultInstrumentationOptions := flagutil.InstrumentationOptions{
MetricsPort: prowflagutil.DefaultMetricsPort,
Expand Down
4 changes: 2 additions & 2 deletions prow/cmd/deck/job_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func getBuildData(ctx context.Context, bucket storageBucket, dir string) (buildD
if finished.Timestamp != nil {
b.Duration = time.Unix(*finished.Timestamp, 0).Sub(b.Started)
} else {
b.Duration = time.Now().Sub(b.Started).Round(time.Second)
b.Duration = time.Since(b.Started).Round(time.Second)
}
if finished.Result != "" {
b.Result = finished.Result
Expand Down Expand Up @@ -484,7 +484,7 @@ func getJobHistory(ctx context.Context, url *url.URL, cfg config.Getter, opener
tmpl.Builds[b.index] = b
}

elapsed := time.Now().Sub(start)
elapsed := time.Since(start)
logrus.Infof("loaded %s in %v", url.Path, elapsed)
return tmpl, nil
}
6 changes: 3 additions & 3 deletions prow/cmd/deck/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ func TestTide(t *testing.T) {
if err != nil {
t.Fatalf("Marshaling: %v", err)
}
fmt.Fprintf(w, string(b))
fmt.Fprint(w, string(b))
}))
ca := &config.Agent{}
ca.Set(&config.Config{
Expand Down Expand Up @@ -661,7 +661,7 @@ func TestTideHistory(t *testing.T) {
if err != nil {
t.Fatalf("Marshaling: %v", err)
}
fmt.Fprintf(w, string(b))
fmt.Fprint(w, string(b))
}))

ta := tideAgent{
Expand Down Expand Up @@ -718,7 +718,7 @@ func TestHelp(t *testing.T) {
if err != nil {
t.Fatalf("Marshaling: %v", err)
}
fmt.Fprintf(w, string(b))
fmt.Fprint(w, string(b))
}))
ha := &helpAgent{
path: s.URL,
Expand Down
2 changes: 1 addition & 1 deletion prow/cmd/deck/pr_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func getPRHistory(ctx context.Context, prHistoryURL *url.URL, config *config.Con
}
}

elapsed := time.Now().Sub(start)
elapsed := time.Since(start)
logrus.WithField("duration", elapsed.String()).Infof("loaded %s", prHistoryURL.Path)

return template, nil
Expand Down
6 changes: 0 additions & 6 deletions prow/cmd/deck/tide.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,10 @@ func (ta *tideAgent) filterHiddenHistory(hist map[string][]history.Record) map[s
filtered := make(map[string][]history.Record, len(hist))
for pool, records := range hist {
needsHide := matches(strings.Split(pool, ":")[0], ta.hiddenRepos())
var ignored []string
if needsHide && ta.showHidden {
filtered[pool] = records
} else if needsHide == ta.hiddenOnly {
filtered[pool] = records
} else {
ignored = append(ignored, pool)
}
}
return filtered
Expand All @@ -211,13 +208,10 @@ func (ta *tideAgent) filterHiddenQueries(queries []config.TideQuery) []config.Ti
break
}
}
var ignored []string
if includesHidden && ta.showHidden {
filtered = append(filtered, qc)
} else if includesHidden == ta.hiddenOnly {
filtered = append(filtered, qc)
} else {
ignored = append(ignored, qc.Query())
}
}
return filtered
Expand Down
2 changes: 1 addition & 1 deletion prow/cmd/phaino/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func processJobs(ctx context.Context, opt options, pjs <-chan prowapi.ProwJob, e
start := time.Now()
log := logrus.WithField("job", jobName(pj))
err := convertJob(ctx, log, pj, opt.priv, opt.printCmd, opt.timeout, opt.grace)
log = log.WithField("duration", time.Now().Sub(start))
log = log.WithField("duration", time.Since(start))
if err != nil {
log.WithError(err).Error("FAIL")
if !opt.keepGoing {
Expand Down
7 changes: 4 additions & 3 deletions prow/cmd/tackle/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@ package main

import (
"bytes"
"testing"
"time"

extensions "k8s.io/api/extensions/v1beta1"
networking "k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
discoveryFake "k8s.io/client-go/discovery/fake"
"k8s.io/client-go/kubernetes"
k8sFake "k8s.io/client-go/kubernetes/fake"
"testing"
"time"
)

func createExtensionsIngressList() *extensions.IngressList {
Expand Down Expand Up @@ -154,7 +155,7 @@ func TestToNewIngress(t *testing.T) {
t.Errorf("Unexpected error marshalling networking ingress: %v", err)
}

if bytes.Compare(oldBytes, newBytes) != 0 {
if !bytes.Equal(oldBytes, newBytes) {
t.Errorf("Expected marshalling of types should be equal")
}
}
6 changes: 1 addition & 5 deletions prow/cmd/tackle/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ func printArray(collection []string, limit int) bool {

// validateNotEmpty handles validation that a collection is non-empty.
func validateNotEmpty(collection []string) bool {
if len(collection) > 0 {
return true
}

return false
return len(collection) > 0
}

// validateContainment handles validation for containment of target in collection.
Expand Down
5 changes: 1 addition & 4 deletions prow/config/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,7 @@ func (br Brancher) Intersects(other Brancher) bool {
baseBranches := sets.NewString(br.Branches...)
if len(other.Branches) > 0 {
otherBranches := sets.NewString(other.Branches...)
if baseBranches.Intersection(otherBranches).Len() > 0 {
return true
}
return false
return baseBranches.Intersection(otherBranches).Len() > 0
}

// Actually test our branches against the other brancher - if there are regex skip lists, simple comparison
Expand Down
Loading

0 comments on commit 04deac2

Please sign in to comment.