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

Remove support for running mixer commands inside container #528

Closed
jwakre opened this issue Jan 2, 2019 · 12 comments
Closed

Remove support for running mixer commands inside container #528

jwakre opened this issue Jan 2, 2019 · 12 comments

Comments

@jwakre
Copy link

jwakre commented Jan 2, 2019

Mixer is no longer required to run commands inside of a container and removing this feature reduces mixer's complexity. After this change, mixer would no longer need the '--native' flag because it will always run natively.

@jwakre jwakre self-assigned this Jan 2, 2019
@rchiossi
Copy link
Contributor

rchiossi commented Jan 2, 2019

In order to remove docker, we will need to check all formats that had this feature available for auto-bump (24, 25, and 26) and make sure mixer can do a proper bump. 25 to 26 should be good to go, but we need to check the changes from 24 to 25.

@jwakre
Copy link
Author

jwakre commented Jan 4, 2019

I don't think removing Docker would impact specific formats, but an incorrect implementation could break format bumps in general. Just to be safe, I ran format bumps from 24->25 and did not experience any problems.

@rchiossi
Copy link
Contributor

rchiossi commented Jan 4, 2019

The reason I mentioned the bump from 24 to 25 is because right now mixer is able to work on those formats by downloading the correct image with the correct mixer version and working inside docker. In order to keep backwards compatibility, the new mixer has to be able to do it as well.

@jwakre
Copy link
Author

jwakre commented Jan 4, 2019

My PR completely removes support for running inside of a container, so older formats would not run in a container. I don't see why older formats would be required to be run inside a container. Executing commands in a container shouldn't modify how manifests are formatted and native builds should have been supported.

@rchiossi
Copy link
Contributor

rchiossi commented Jan 4, 2019

The only reason containers exists in the first place is to add support for older formats. This way mixer could focus only in the current format and download older versions of itself to generate content for older format. This means that up until support for format 26 was added, in paper mixer was unable to process older formats. If it works, it is by pure chance.

@jwakre
Copy link
Author

jwakre commented Jan 4, 2019

Ok, I was thinking that containers were a security requirement that was removed.

So, if I understand you correctly:

The current version of Mixer recently started using format_switch.go to support other formats, but this is limited to go back as far as format 26. Historically, Mixer built other formats inside a container with an old release of Mixer.

@jwakre
Copy link
Author

jwakre commented Jan 4, 2019

In that case, I see the format issue now.

@rchiossi
Copy link
Contributor

rchiossi commented Jan 4, 2019

This feature was introduced to handle format bumps. When doing a bump, you need to generate two sets of metadata: One with the tooling for the old format and one with the tooling of the new format. As such, mixer would download two docker images, one with the latest mixer version for the old format and one with the latest mixer version for the new format. Then it would generate the update metadata twice, once with each downloaded version. For a format bump without docker, like what devops do, they actually had to manually download two versions of mixer and manually swap versions to generate that metadata.

Format 26 was the first format that didn't require this change since the mixer binary was able to generate metadata for both format 25 and 26, removing the need to manually swap binaries during format bumps.

Besides its use for format bumps, using docker had the advantage of setting an unified build environment, which made it easier to run mixes on other systems other than clear and that is why it was made default. Its original design was solely focused on supporting older formats for bumps though.

@jwakre
Copy link
Author

jwakre commented Jan 4, 2019

Based on our conversation, I can re-define this enhancement as follows:

  • formats >= 25, build natively on host always.
  • formats < 25 build in container by default and natively when passed --native flag. Although, maybe it would be better to force these old formats to always run inside their respective containers. This way the native flag could be deprecated.

This would break users with incompatible environments who expect Mixer to run in a container. If this is important, we could drop the PR, but I think there are more benefits for not using a container and these users could make their own container to run Mixer.

@gtkramer
Copy link

gtkramer commented Oct 30, 2019

Removing the docker component/--native flag to mixer would help improve the overall CLI for build commands, and ties in with the CLI improvement efforts proposed in #627.

@rchiossi
Copy link
Contributor

We don't have any blocker to proceed with this now. Support for format 25 and below is not something we need to worry anymore and for users using docker for compatibility only, we should point them to mixer-ci image instead.

@reaganlo
Copy link
Contributor

Fixed in v6.0.0

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

4 participants