Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.travis.yml: Prune moved-to-Prow tf-lint, etc. #123

Merged

Conversation

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 14, 2018
@wking
Copy link
Member Author

wking commented Aug 14, 2018

The build-tarball error was:

+ bazel --output_base=/tmp build tarball
Error: $USER is not set, and unable to look up name of current user: (error: 0): Success

Maybe the Prow config for that job is broken? Looks like it just landed four hours ago.

@wking
Copy link
Member Author

wking commented Aug 14, 2018

/hold

I've pushed a WIP commit to set the USER. If that fixes the Bazel failure, I'll update the Prow config appropriately. Until then, we don't want to merge this PR.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2018
@wking wking force-pushed the prune-prow-from-travis branch 4 times, most recently from ff7af74 to e4740aa Compare August 14, 2018 19:56
@wking
Copy link
Member Author

wking commented Aug 14, 2018

/hold cancel

I've dropped the WIP testing commit now that I've spun off the tarball fixes into and openshift/release#1185 and #125.

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2018
These have moved into Prow presubmit jobs:

* Terraform lint: openshift/release@82e00346 (Prow: Add Terraform Lint
  to openshift/installer, 2018-08-06, openshift/release#1124).
* YAML lint: openshift/release@457be2cd (Added prow yaml-lint job
  description for installer repo, 2018-08-02, openshift/release#1138).
* ShellCheck: openshift/release@e12a7a06 (Prow: Add shellcheck to
  openshift/installer, 2018-08-08, openshift/release#1131).
* Terraform format: openshift/release@f67e06e4 (openshift installer:
  add terraform fmt, 2018-08-04, openshift/release#1152).
* Go vet: openshift/release@71afdcca (Added go-vet prow job,
  2018-08-14, openshift/release#1181).
* Building the tarball: openshift/release@42a5a0d0 (add
  openshift/installer 'bazel build tarball' test to prow, 2018-08-13,
  openshift/release#1178).
@wking
Copy link
Member Author

wking commented Aug 14, 2018

I've pushed e4740aa -> 1765e93 to also drop go-vet, now that openshift/release@71afdcca (openshift/release#1181) has landed.

@wking
Copy link
Member Author

wking commented Aug 14, 2018

/retest

@wking
Copy link
Member Author

wking commented Aug 14, 2018

All green :).

/assign @abhinavdahiya

The remaining Go lint Travis job is being addressed by #118 and openshift/release#1177. The Go unit test job has already been partially addressed with #97, although I'm not aware of an analogous release PR for gotest.sh. But I don't think we need to wait on those before landing this.

@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sallyom
Copy link
Contributor

sallyom commented Aug 15, 2018

@wking the go unit test is also moved to prow already, it's running within ci-operator here: https://github.com/openshift/release/blob/master/ci-operator/jobs/openshift/installer/openshift-installer-presubmits.yaml#L3 and with this master.json: https://github.com/openshift/release/blob/master/ci-operator/config/openshift/installer/master.json#L44
(installer/hack/gotest.sh was merged in before we realized the ci/prow/unit was already configured to run go test.. we can remove gotest.sh, it is not required for use in prow. I can open a PR to remove hack/gotest.sh or you can remove it with this PR, as well as the task in the .travis.yaml)

@wking
Copy link
Member Author

wking commented Aug 15, 2018

The e2e-aws errors were:

Error: Error refreshing state: 2 error(s) occurred:

* module.assets_base.module.bootkube.data.ignition_file.bootkube_sh: 1 error(s) occurred:

* module.assets_base.module.bootkube.data.ignition_file.bootkube_sh: data.ignition_file.bootkube_sh: unexpected EOF
* module.assets_base.data.ignition_config.etcd: 3 error(s) occurred:

* module.assets_base.data.ignition_config.etcd[2]: data.ignition_config.etcd.2: unexpected EOF
* module.assets_base.data.ignition_config.etcd[1]: data.ignition_config.etcd.1: unexpected EOF
* module.assets_base.data.ignition_config.etcd[0]: data.ignition_config.etcd.0: unexpected EOF

Not sure what's going on there.

/retest

@openshift-merge-robot openshift-merge-robot merged commit 1a06e47 into openshift:master Aug 15, 2018
@wking wking deleted the prune-prow-from-travis branch August 15, 2018 16:29
@wking
Copy link
Member Author

wking commented Aug 15, 2018

... the go unit test is also moved to prow already, it's running within ci-operator here...

Ah, thanks for the pointers. I've filed #135 to drop the script.

wking added a commit to wking/openshift-installer that referenced this pull request Aug 27, 2018
* Drop the go-lint comment from go-fmt.sh.  I'd accidentally
  copy/pasted this over in 87b3e17 (hack/go-fmt: Make it easy to
  auto-format Go, 2018-08-24, openshift#173).
* Suggest users run go-lint over pkg/... as well.

* Simplify the 'go vet' invocation.  The code I'm removing is from
  eb71c8d (Added go-vet shell script, 2018-08-06, openshift#110).  The Travis
  config it replaced was removed in 1765e93 (.travis.yml: Prune
  moved-to-Prow tf-lint, etc., 2018-08-14, openshift#123), but was just
  running:

    go vet ./installer/...

  inside the container.  So I don't think we need the directory
  changing or moving here.  And we should certainly be able to cover
  the test from a single container, instead of launch a new container
  for each requested directory (at least as long as the requested
  directories are under $PWD, and the old script required that
  anyway).
wking added a commit to wking/openshift-installer that referenced this pull request Aug 27, 2018
Catching up with 4e92db8 (Merge pull request openshift#120 from
staebler/asset_graph_engine, 2018-08-24).

* Drop the go-lint comment from go-fmt.sh.  I'd accidentally
  copy/pasted this over in 87b3e17 (hack/go-fmt: Make it easy to
  auto-format Go, 2018-08-24, openshift#173).
* Suggest users run go-lint over pkg/... as well.

* Simplify the 'go vet' invocation.  The code I'm removing is from
  eb71c8d (Added go-vet shell script, 2018-08-06, openshift#110).  The Travis
  config it replaced was removed in 1765e93 (.travis.yml: Prune
  moved-to-Prow tf-lint, etc., 2018-08-14, openshift#123), but was just
  running:

    go vet ./installer/...

  inside the container.  So I don't think we need the directory
  changing or moving here.  And we should certainly be able to cover
  the test from a single container, instead of launch a new container
  for each requested directory (at least as long as the requested
  directories are under $PWD, and the old script required that
  anyway).
EmilienM pushed a commit to shiftstack/installer that referenced this pull request Dec 8, 2020
EmilienM pushed a commit to shiftstack/installer that referenced this pull request Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants