Skip to content

PROPOSAL: Reorganize pkg/skaffold directory #1765

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

Closed
1 of 13 tasks
priyawadhwa opened this issue Mar 8, 2019 · 3 comments
Closed
1 of 13 tasks

PROPOSAL: Reorganize pkg/skaffold directory #1765

priyawadhwa opened this issue Mar 8, 2019 · 3 comments
Labels
internal kind/todo implementation task/epic for the skaffold team

Comments

@priyawadhwa
Copy link
Contributor

priyawadhwa commented Mar 8, 2019

This proposal will outline a process for reorganizing the skaffold pkg directory so that it's cleaner & better organized with respect to plugin builders.

Current Directory Structure

The current directory structure looks like this (I removed packages that won't be changing):

pkg/skaffold
  ├── bazel
  ├── build
  │   ├── bazel
  │   ├── docker
  │   ├── kaniko
  │   │   └── sources
  │   ├── local
  │   ├── plugin
  │   └── tag
  ├── plugin
      ├── builders
      │   ├── bazel
      │   └── docker
      ├── environments
      │   └── gcb
      └── shared

Issues with the current structure:

  • We have bazel, build/bazel, and plugin/builders/bazel directories, and it's unclear what the difference is. These should all be consolidated into one.
  • We have a plugin/environments/gcb environment, so we should eventually also have plugin/environments/local and plugin/environments/incluster. These environments should be build tool agnostic (as in, googleCloudBuild should contain functions that can be used by jib and docker builders)
  • pkg/skaffold/build/tag should just be pkg/skaffold/tag since tagging has been decoupled from building
  • build/plugin contains the PluginBuilder, which should be renamed to ArtifactBuilder and should live somewhere else.

Goal Directory Structure

pkg/skaffold
    ├── build
    │   ├── cache
    │   ├── bazel
    │   ├── docker
    │   ├── jib
    │   └── kaniko
    │       └── sources
    │   ├── environments
    │   │   └── gcb
    │   │   └── local
    │   │   └── incluster
    │   ├── plugin # whatever is in pkg/plugin/shared currently + PluginBuilder implementation 
    │       └── builder.go # PluginBuilder -> ArtifactBuilder that should be build.Builder 
    |── tag

Changes:

  • The current pkg/skaffold/plugin directory will move into pkg/skaffold/build. In that, we will have directories for each core build tool (kaniko/jib/docker/bazel), and all environments.
  • pkg/skaffold/plugin will hold everything currently in pkg/skaffold/plugin/shared & the ArtifactBuilder type
  • tag will be moved out of build

How we'll get here
The goal directory structure can only be achieved once all core plugins have been implemented and can replace the old functionality.

With that in mind, I'll be submitting PRs for the following checklist in order to get here:

  • 1. Move as much build code into plugins directory as possible
    • Refactor bazel builder by moving pkg/bazel, pkg/build/local/bazel.go and associated tests into plugin/builders/bazel
    • Refactor docker builder similarly to bazel builder
    • Refactor kaniko builder similarly to bazel builder
    • Refactor jib builder similarly to bazel builder
    • Refactor environments pkg to be build tool agnostic
  • 1a. move cache.go to pkg/build/cache
  • 1b. separate plugin client and server code
  • 2. Move build/tag to tag
  • 3. Once core plugins are robust, move pkg/plugins to pkg/build
    • Rename plugin/shared to build/plugin
    • Move plugin/builder/<kaniko/jib/docker/bazel> to build/<kaniko/jib/docker/bazel>
    • Rename PluginBuilder to ArtifactBuilder and move to build/plugin/builder.go
@nkubala
Copy link
Contributor

nkubala commented Mar 11, 2019

this looks great. one other thing I'd like to see here is separation of client and server code for the native plugins. I know the interface receiver for the methods should tell me what's what, but this can sometimes get confusing when working with them which component I'm actually modifying. apart from that I'm a big fan of this

@priyawadhwa
Copy link
Contributor Author

@nkubala thanks for the feedback! I've added that to the list of requirements.

@balopat
Copy link
Contributor

balopat commented Aug 15, 2019

We can close this as it was mainly around the plugin structure that got removed.

@balopat balopat closed this as completed Aug 15, 2019
@balopat balopat added the kind/todo implementation task/epic for the skaffold team label Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal kind/todo implementation task/epic for the skaffold team
Projects
None yet
Development

No branches or pull requests

4 participants