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

[K8S] Fix issue in 'docker-image-tool.sh' #21551

Closed
wants to merge 1 commit into from

Conversation

fabriziocucci
Copy link

@fabriziocucci fabriziocucci commented Jun 13, 2018

Because of the missing assignment of the variable BUILD_ARGS the command ./bin/docker-image-tool.sh -r docker.io/myrepo -t v2.3.1 build fails:

docker build" requires exactly 1 argument.
See 'docker build --help'.

Usage:  docker build [OPTIONS] PATH | URL | - [flags]

Build an image from a Dockerfile

This has been fixed on the master already but, apparently, it has not been ported back to the branch 2.3, leading to the same error even on the latest 2.3.1 release (dated 8 June 2018).

Because of the missing assignment of the variable `BUILD_ARGS` the command `./bin/docker-image-tool.sh -r docker.io/myrepo -t v2.3.0 build` fails:

```
docker build" requires exactly 1 argument.
See 'docker build --help'.

Usage:  docker build [OPTIONS] PATH | URL | - [flags]

Build an image from a Dockerfile
```

This has been fixed on the `master` already but, apparently, it has not been ported back to the branch `2.3`, leading to the same error even on the latest `2.3.1` release (dated 8 June 2018).
@felixcheung
Copy link
Member

ouch

@felixcheung
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jun 16, 2018

Test build #91967 has finished for PR 21551 at commit 9e52496.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

@ifilonenko ifilonenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @mccheah @foxish to merge

@mccheah
Copy link
Contributor

mccheah commented Jun 18, 2018

@rxin for SA that this is going into branch-2.3. Should be fine - we can ship this if/when we cut 2.3.2. Merging.

asfgit pushed a commit that referenced this pull request Jun 18, 2018
Because of the missing assignment of the variable `BUILD_ARGS` the command `./bin/docker-image-tool.sh -r docker.io/myrepo -t v2.3.1 build` fails:

```
docker build" requires exactly 1 argument.
See 'docker build --help'.

Usage:  docker build [OPTIONS] PATH | URL | - [flags]

Build an image from a Dockerfile
```

This has been fixed on the `master` already but, apparently, it has not been ported back to the branch `2.3`, leading to the same error even on the latest `2.3.1` release (dated 8 June 2018).

Author: Fabrizio Cucci <fabrizio.cucci@gmail.com>

Closes #21551 from fabriziocucci/patch-1.
@mccheah
Copy link
Contributor

mccheah commented Jun 18, 2018

Missed this before merging, but @fabriziocucci for future reference please put the ticket number along with [K8S] in the PR description. Sorry I didn't catch this before merging.

@fabriziocucci
Copy link
Author

@mccheah apologies for not following the contribution guidelines here.

I'm not sure there is a ticket number for this (I didn't create it and the closest issue I've found is this but it's closed already).

Do you want me to create one?

@fabriziocucci fabriziocucci changed the title Fix issue in 'docker-image-tool.sh' [K8S] Fix issue in 'docker-image-tool.sh' Jun 18, 2018
@mccheah
Copy link
Contributor

mccheah commented Jun 18, 2018

The PR is already merged so it's too late now. Next time, please create a ticket if one does not exist and put it in the PR description.

@fabriziocucci
Copy link
Author

Duly noted! 👍

@erikerlandson
Copy link
Contributor

@mccheah this isn't showing as merged, you said you ran the merge script?

@mccheah
Copy link
Contributor

mccheah commented Jun 18, 2018

Hm, I thought I did. I don't quite know what's going on.

@mccheah
Copy link
Contributor

mccheah commented Jun 18, 2018

asfgit is showing the commit as pushed above.

@erikerlandson
Copy link
Contributor

OK, I see it referenced above as commit b8dbfcc. I'd close this, but that option is disabled for me. Probably assuming it happens from the merge-pr script.

@erikerlandson
Copy link
Contributor

@fabriziocucci can you close this PR?

@vanzin
Copy link
Contributor

vanzin commented Jun 19, 2018

FYI, PRs against non-master branches need to be manually closed. Don't know the exact explanation for why that's the case.

@foxish
Copy link
Contributor

foxish commented Jun 20, 2018

Phew, this was a bad one. Thanks for the fix all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants