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

Makisu plugin registry_config parameter has no effect #257

Closed
colindean opened this issue Apr 28, 2021 · 4 comments
Closed

Makisu plugin registry_config parameter has no effect #257

colindean opened this issue Apr 28, 2021 · 4 comments
Labels
bug Indicates a bug

Comments

@colindean
Copy link

Description

It's a ~known problem that Makisu has issues pushing to Artifactory without setting some registry config settings:

docker.example.com:
          .*:
            push_chunk: -1
            timeout: 1800s

The registry-config parameter for the Makisu plugin is confusing and not working as intended by the fix in go-vela/vela-makisu#33. It was intended to wire in the interface to where it's used, but I misunderstood how Build.RegistryConfig is used.

At the high level, it seems like it was intended to be JSON that eventually gets merged into some hardcoded config. Looking more deeply, it seems like maybe it was originally intended to be a path to a file, overriding the default path in the code.

But, it wasn’t wired up because there’s no code path that reads the parameter and puts it into the Registry struct. It uses the hardcoded one no matter what.

Hardcoded value: https://github.com/go-vela/vela-makisu/blob/07862e4d408b920310f75a416b7bdcfc3f744019/cmd/vela-makisu/registry.go#L123

Use, overwriting what the user provides: https://github.com/go-vela/vela-makisu/blob/07862e4d408b920310f75a416b7bdcfc3f744019/cmd/vela-makisu/plugin.go#L48

Writing after the plugin struct has been populated from the parameters: https://github.com/go-vela/vela-makisu/blob/07862e4d408b920310f75a416b7bdcfc3f744019/cmd/vela-makisu/plugin.go#L36

Value

I need to be able to set registry config settings in order for my containers built with Makisu to push using Makisu. Without this, Makisu doesn't push correctly and then Docker can't pull the image's layers from Artifactory.

Useful Information

  1. What is the output of vela --version?

Whatever's running in prod internally…?

  1. What operating system is being used?

Linux

Proposed Solution

  1. Rename Build.RegistryConfig to Build.RegistryConfigPath and enable the user to set that explicitly, ignoring all of the stuff built into the plugin that constructs the file from hardcoded defaults/templates. It'd be an advanced option!
  2. Create a Build.ExtraRegistryConfig / extra_registry_config parameter that works like I thought registry_config worked: merge configuration provided in that parameter with what's constructed from hardcoded defaults/templates.
  3. Remove references/usages of registry_config from parameters, because it never actually did anything anyway.
@colindean colindean added the bug Indicates a bug label Apr 28, 2021
@colindean
Copy link
Author

I should note that with Kaniko inadequate for our use case because of GoogleContainerTools/kaniko#1375 (including an entry for / in the layer tar) and without being able to specify the registry_config for Makisu, we may not be able to use Vela to build/push our containers.

@susangreve
Copy link

Thanks Colin! We are discussing this as a team

@colindean
Copy link
Author

N.b. a newer version of the software consuming the output of our Docker builds changed how it reads the container filesystem in a way that is now compatible with Kaniko. We switched to Kaniko from Makisu and this issue is no longer relevant to us.

Given that Makisu development ended (#258), this bug is probably no longer worth considering fixing.

@wass3r
Copy link
Collaborator

wass3r commented Sep 2, 2022

i will close this since we stopped supporting this plugin.

@wass3r wass3r closed this as completed Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates a bug
Projects
None yet
Development

No branches or pull requests

3 participants