Skip to content

CentOS 7 docker based image #1571

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

Merged
merged 7 commits into from
Mar 4, 2019

Conversation

mr-exz
Copy link
Contributor

@mr-exz mr-exz commented Jan 19, 2019

added Dockerfile with:

  • clear cenots/7
  • dotnet core 2.2
  • gitversion 4.0.0

added Dockerfile with clear cenots/7,dotnet core 2.2, gitversion 4.0.0
@asbjornu
Copy link
Member

asbjornu commented Jan 21, 2019

Thanks for this, @mr-exz. Is it possible to add an automatic test that is run by Travis or similar, somehow?

@arturcic
Copy link
Member

@mr-exz thanks for the contribution, we currently publish the docker images as part of the build process. In order to get this PR merged we need to change the way the docker image is built, meaning the image should use the artifact from the build itself and not from Github release, that means we need to include in the build scripts. Currently we have 2 variants of the linux images - one using dotnet core and another using mono. Would be great to have the mono version as well.

For the build scripts we'll need to support building for different distros of Linux here and here

So for now I would suggest to update the build script to allow building for several distro of Linux, then adjust the DockerFile to use the artifacts from the build itself

@mr-exz
Copy link
Contributor Author

mr-exz commented Jan 21, 2019

Thanks for this, @mr-exz. Is it possible to add an automatic test that is run by Travis or similar, somehow?

i will check how you did it, and will try fit my Dockerfile into build process

@mr-exz
Copy link
Contributor Author

mr-exz commented Jan 21, 2019

@asbjornu @arturcic pls check changes

build.cake Outdated
@@ -622,6 +623,7 @@ Task("Publish-DockerHub")
{
DockerPush("linux", "dotnetcore", parameters);
DockerPush("linux", "fullfx", parameters);
DockerPush("linux", "centos", parameters);
Copy link
Member

@asbjornu asbjornu Jan 22, 2019

Choose a reason for hiding this comment

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

Please excuse my ignorance, but how is this going to affect the naming on DockerHub? I don't quite understand where the existing repositories gitversion-fullfx, gitversion-dotnetcore, gitversion and libgit2sharp-mono come from and how they relate to these DockerPush() statements. Will centos become a new tag in the gitversion-dotnetcore repository?

Copy link
Member

Choose a reason for hiding this comment

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

@asbjornu gitversion and libgit2sharp-mono are already obsolete, only fullfx and dotnetcore are maintained.

As for centos, that is right, the current changes will not create a tag under dotnetcore - good catch, we'll need to adjust the scripts to allow several distros of linux under dotnetcore and fullfx

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@asbjornu asbjornu Jan 22, 2019

Choose a reason for hiding this comment

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

I see. I find it a bit unfortunate that we have 4 different repositories for variations of the same executable. Can we get rid of every repository that isn't called simply gitversion and follow NGINX' naming scheme for labels to distinguish different versions of the image? That would give us one repository with tags following the scheme {version}-{platform}-{framework}, for example:

  • 4.0.0-centos
  • 4.0.1-beta2-debian-fx
  • 4.0.0-beta15-windows-dotnetcore
  • latest-ubuntu
  • latest-windows
  • stable-ubuntu
  • stable-windows

I know doing anything about this scheme is outside the scope of this PR, I just want to have a discussion on this so I know which options we have.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest creating a separate issue, I can then implement it, and after that we can adjust this PR to the new scheme.

Copy link
Member

Choose a reason for hiding this comment

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

@asbjornu please create an issue with the proposed scheme and we can discuss it there

Copy link
Member

Choose a reason for hiding this comment

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

Sure, I've created #1572 for further discussion.

@asbjornu asbjornu mentioned this pull request Jan 22, 2019
@asbjornu asbjornu added this to the 5.0.0 milestone Feb 20, 2019
@arturcic
Copy link
Member

@asbjornu can we postpone merging this till I get #1572 implemented ?

@asbjornu
Copy link
Member

@arturcic, yep, that's the plan. 😃

@arturcic
Copy link
Member

@mr-exz the #1572 was implemented and merged, can you rebase on top of master and add the CentOS support? Do you think we can also add Fedora support?

@mr-exz
Copy link
Contributor Author

mr-exz commented Feb 26, 2019

@mr-exz the #1572 was implemented and merged, can you rebase on top of master and add the CentOS support? Do you think we can also add Fedora support?

i think we can, i'll take a look

@arturcic
Copy link
Member

arturcic commented Mar 4, 2019

@mr-exz , I think you should move the Dockerfile to the src/Docker/linux/centos/netcoreapp2.1 folder similar to debian version. This way we'll be able to add net core 2.2 version as well

@arturcic
Copy link
Member

arturcic commented Mar 4, 2019

Looks great! Thank you for your contribution!

@arturcic arturcic merged commit b6e2fdf into GitTools:master Mar 4, 2019
@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 4, 2019

Not at all

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 5, 2019

@arturcic can you provide link on new centos7/fedora27 docker images link ?

@arturcic
Copy link
Member

arturcic commented Mar 5, 2019

Yesterday when the PR was merged the Linux build has failed, some unit tests for some reason failed, now they work again. The tags can be found at https://hub.docker.com/r/gittools/gitversion/tags

@arturcic
Copy link
Member

arturcic commented Mar 5, 2019

@mr-exz can you confirm that those docker images are valid and work as expected as I don't have time to test them these days?

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 5, 2019

i will, in process

@arturcic
Copy link
Member

arturcic commented Mar 5, 2019

Thanks

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 5, 2019

For me working fine

# docker run --rm -v "$(pwd):/repo" gittools/gitversion:5.0.0-linux-centos7-netcoreapp2.1 /repo
# docker run --rm -v "$(pwd):/repo" gittools/gitversion:5.0.0-linux-fedora27-netcoreapp2.1 /repo

@arturcic
Copy link
Member

arturcic commented Mar 5, 2019

great, thanks for the fedora image as well.

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 5, 2019

Not at all :)

@arturcic
Copy link
Member

arturcic commented Mar 5, 2019

Btw, are you in a position to add a new PR for netcoreapp2.2? for linux images? That would be a great addition

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 5, 2019

ok will do, it shouldn't be so difficult

@asbjornu
Copy link
Member

asbjornu commented Mar 6, 2019

Hm, I just discovered that the tag gittools/gitversion:5.0.0-linux-centos7-netcoreapp2.1 has the version number 5.0.0 which indicates it’s a stable version. Since it isn’t, that’s a problem, no? Can we get the pre-release label into there so we only tag stable releases as such?

@arturcic
Copy link
Member

arturcic commented Mar 6, 2019

There is also a tag gittools/gitversion:5.0.0-beta2-18-linux-centos7-netcoreapp2.1 that is the same as the one you mentioned. Basically on every new build we re-tag the 5.0.0-linux-centos7-netcoreapp2.1 to the newest version, as well as we tag with a pre-release tag. That means the pre-release tag will not change, but the the other tag gets updated with every new commit. (it's is similar to how latest tag is used in other projects, meaning it's always pointing to the latest, stable or not stable)

@arturcic
Copy link
Member

arturcic commented Mar 6, 2019

@arturcic
Copy link
Member

arturcic commented Mar 6, 2019

@mr-exz
Copy link
Contributor Author

mr-exz commented Mar 9, 2019

Btw, are you in a position to add a new PR for netcoreapp2.2? for linux images? That would be a great addition

#1633

@asbjornu
Copy link
Member

@arturcic, thanks for the explanation. That sounds sensible to me. 👍

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

Successfully merging this pull request may close these issues.

3 participants