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

Resolve issues to finish Jib builder. #1096

Closed
11 tasks done
coollog opened this issue Oct 4, 2018 · 17 comments
Closed
11 tasks done

Resolve issues to finish Jib builder. #1096

coollog opened this issue Oct 4, 2018 · 17 comments

Comments

@coollog
Copy link
Contributor

coollog commented Oct 4, 2018

/cc @GoogleContainerTools/java-tools

@briandealwis
Copy link
Member

  • Build directly to registry when skipPush is true

The local builder handles this already. I don't think there's anything for us to do here.

@briandealwis
Copy link
Member

I'm reworking the wrapper logic.

@saturnism
Copy link

Quick question.. if skipPush: true, then it will build directly to registry (which is likely remote), what does skipPush: false mean?

@coollog
Copy link
Contributor Author

coollog commented Oct 8, 2018

That was actually supposed to be when skipPush is false. Fixed it. When skipPush is true, Jib will build to the Docker daemon.

@balopat
Copy link
Contributor

balopat commented Oct 8, 2018

FYI. I added #1114 which will invert SkipPush to Push.

@coollog
Copy link
Contributor Author

coollog commented Oct 8, 2018

The logic for handling push=true seems to be not as trivial as originally thought, mainly due to the fact that 3 out of the 5 taggers require the image digest to generate the intended tag (ChecksumTagger, GitCommit, and envTemplateTagger).

local.go#buildArtifact performs a series of steps for a build:

  1. b.runBuildForArtifact builds to the Docker daemon and gets the initial tag
  2. docker.Digest gets the digest for the initial tag from the Docker daemon
  3. tagger.GenerateFullyQualifiedImageName generates the intended tag
  4. b.api.ImageTag retags the initial tag to the intended tag
  5. if push=true, docker.RunPush pushes the intended tag

For Jib build, if push=false, the above series of steps works fine. However, for push=true, the steps should ideally be:

  1. b.runBuildForArtifact runs jib:_skaffoldDigest (nonexistent) to get the digest of the image to be built
  2. tagger.GenerateFullyQualifiedImageName generates the intended tag
  3. Run jib:build to build to the intended tag

The above steps require adding the jib:_skaffoldDigest goal/task. A workaround for now could be:

  1. b.runBuildForArtifact runs jib:build and then docker pull on the built image using artifact.ImageName as the initial tag
  2. do rest of steps...

Another alternative workaround is:

  1. b.runBuildForArtifact as is, calling jib:dockerBuild to build to the Docker daemon
  2. ... do other steps up to generating the intended tag
  3. Run jib:build to build to the intended tag

However, these workaround do not capture the full benefit we get from jib:build.

Thoughts? @GoogleContainerTools/java-tools @GoogleContainerTools/container-tools

@loosebazooka
Copy link
Member

I'm assuming this is a problem for the remote builders as well?

@coollog
Copy link
Contributor Author

coollog commented Oct 8, 2018

Filed #1124 with a proposal for how to handle build to registry.

@dgageot
Copy link
Contributor

dgageot commented Oct 9, 2018

I've got a few questions on current implementation. None of which is blocking:

  • Is there a way to add a single type of artifact called Jib that auto-detects if it's a gradle or a maven build? That would make the config easier to read/write. I know the maven and gradle configs don't have the same parameters though.
  • Does Jib require Skaffold to generate an image name with generateJibImageRef? Other builder just use a random string.

@coollog
Copy link
Contributor Author

coollog commented Oct 9, 2018

Is there a way to add a single type of artifact called Jib that auto-detects if it's a gradle or a maven build? That would make the config easier to read/write. I know the maven and gradle configs don't have the same parameters though.

Yep, we can definitely auto-detect Gradle/Maven (via detection of pom.xml or build.gradle or the respective wrappers). However, the concern is which priority to detect these in. For example, someone could have both pom.xml and build.gradle and those files could be named differently as well. The wrappers could also both be present. In any case, we would also need to provide a user configuration for Maven or Gradle as a fallback in case the auto-detection was incorrect. This detection could probably be specified via a buildFile parameter where passing in an .xml file would mean Maven and .gradle file would be Gradle.

Does Jib require Skaffold to generate an image name with generateJibImageRef? Other builder just use a random string.

Nope, the generateJibImageRef just provides a more readable image name where unless invalid, has the project name as part of the image name (@briandealwis ). Is there a preference for that versus just a random string?

@GoogleContainerTools/java-tools

@dgageot
Copy link
Contributor

dgageot commented Oct 9, 2018

@coollog Maybe have a jib artifact with a type property. Type would be auto-detected unless the user specifies maven or gradle. Once again, it can be changed later but I think it makes the configuration simpler.

I'd use a random string then, it makes the code simpler and more alike the other builders

@briandealwis
Copy link
Member

For the Maven case we need to look for a pom.xml or a .mvn/ directory — Maven now supports a polyglot mode (e.g., using groovy or Scala or other formats for expressing the project model), so pom.xml may not exist.

@loosebazooka
Copy link
Member

Maven and Gradle will probably have different configuration parameters. We might need to make it nested to handle a single jib artifact.

jib {
  maven {
    profile = "asdf"
    ...
  }
}

// or

jib {
  gradle {
    ...
  }
}

@briandealwis
Copy link
Member

briandealwis commented Oct 9, 2018

@dgageot I chose to generate a non-random image name to avoid polluting the local docker daemon's image registry. I generate local images that have some value, so I prefer to avoid having to docker system prune to clean up after skaffold.

Update I should have filed that as a FR though.

@dgageot
Copy link
Contributor

dgageot commented Oct 9, 2018

@briandealwis Does that work? (I honestly haven't thought enough about it)

@briandealwis
Copy link
Member

I can at least filter docker images --format "{{.Repository}}:{{.ID}}" on jib*. It does feel like a losing battle though :-(

@dgageot
Copy link
Contributor

dgageot commented Oct 9, 2018

For me setting a meaningful name has very few advantages. I won't fight against it but I'm all for a simpler codebase.

I'm also ok to make image pruning a P0

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

No branches or pull requests

6 participants