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.

Sign up for GitHub

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

Jump to bottom

Make it work for CentOS7/CentOS8/RHEL7/RHEL8 #793

Merged
merged 2 commits into from
Jul 16, 2020
Merged

Make it work for CentOS7/CentOS8/RHEL7/RHEL8 #793

merged 2 commits into from
Jul 16, 2020

Conversation

kron4eg
Copy link
Member

@kron4eg kron4eg commented Jul 15, 2020

What this PR does / why we need it:
This PR unblocks the ability to user CentOS8 / RHEL7.

  • now it's possible to upgrade docker version
  • CentOS8 / RHEL7 can be provisioned

Special notes for your reviewer:
While by default without externally provided image machine-controller will search for CentOS7 images (fixing this is out of scope of this PR), it's now possible to provide CentOS8 (or RHEL7) image in MachineDeployment.

Optional Release Note:

support CentOS7/CentOS8/RHEL7/RHEL8

@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. labels Jul 15, 2020
@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kron4eg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 15, 2020
@kron4eg kron4eg requested a review from moadqassem July 15, 2020 12:03
@moadqassem
Copy link
Member

moadqassem commented Jul 15, 2020 •
edited
Loading

@kron4eg did you update the e2e tests? I would like to see the tests run for centos 8 where it is supported :). about Kubevirt I can update our registry, but there should be some work done on the cloud providers which we have our own images(vSphere)

@irozzo-1A
Copy link
Contributor

irozzo-1A commented Jul 15, 2020 •
edited
Loading

@kron4eg did you update the e2e tests? I would like to see the tests run for centos 8 where it is supported :). about Kubevirt I can update our registry, but there should be some work done on the cloud providers which we have our own images(vSphere)

I agree, I think we also need tests for RHEL7, and updating this https://github.com/kubermatic/machine-controller/blob/master/docs/operating-system.md#supported-os-versions

@kron4eg kron4eg mentioned this pull request Jul 15, 2020
Merged
@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

@moadqassem not exactly :D I mean, all the tests for centos7 are still valid. It's that now it's possible to add new centos8 scenarios. OK, I'll add some.

kron4eg added 2 commits July 15, 2020 15:21
@kron4eg
Make it work for CentOS7/CentOS8/RHEL7/RHEL8
92c0aa5 
Plus, it's possible to choose whatever version of docker

Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
CentOS8 AWS e2e test
99a4436 
Signed-off-by: Artiom Diomin <kron82@gmail.com>
@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/test pull-machine-controller-e2e-aws-centos8

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

@moadqassem added "special case test" pull-machine-controller-e2e-aws-centos8 modeled after sles

@moadqassem
Copy link
Member

moadqassem commented Jul 15, 2020 •
edited
Loading

@moadqassem added "special case test" pull-machine-controller-e2e-aws-centos8 modeled after sles

We also need one for rhel 7 in azure since we have a customer who is using RHEL on Azure.

Soon we have to think of away how to handle these os versions during testing. At the moment we have the herlper.go file which acts as a customiser for those tests, that file has grown in a very unproductive way and the semantics of how we run things is very confusing. In the close future we need to find a better away to handle the e2e tests and cloud-init injection in general. I have an idea in my head how we can achieve this but we should discuss all to see if it makes sense.

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

We also need one for rhel 7 in azure since we have a customer who is using RHEL on Azure.

From customer point of view nothing will be changed, RHEL8 continues to function like it was.
RHEL7 can wait until we officially have a way for "OS versioning" (centos 7/8, rhel 7/8, ubuntu 18/20), <<- this is a major problem and definitely out of this PR scope :)

@moadqassem
Copy link
Member

/test pull-machine-controller-e2e-kubevirt

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/test pull-machine-controller-yamllint

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/test pull-machine-controller-e2e-vsphere

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/test pull-machine-controller-e2e-vsphere-resource-pool

@moadqassem
Copy link
Member

/test pull-machine-controller-e2e-kubevirt

@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/retest

1 similar comment
@kron4eg
Copy link
Member Author

kron4eg commented Jul 15, 2020

/retest

@moadqassem
Copy link
Member

/lgtm

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 16, 2020
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2017fcb024fc4a3b07d03c7c8d2d0f3a097af2c6

@irozzo-1A
Copy link
Contributor

irozzo-1A commented Jul 16, 2020 •
edited
Loading

LGTM as well, just https://github.com/kubermatic/machine-controller/blob/master/docs/operating-system.md#supported-os-versions has not been updated to include CentOS8, we can do it in a following PR though

xmudrii
xmudrii approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

/lgtm

@xmudrii
Copy link
Member

xmudrii commented Jul 16, 2020

I assume this is a flake.
/retest

@kron4eg
Copy link
Member Author

kron4eg commented Jul 16, 2020

/retest

@kubermatic-bot kubermatic-bot merged commit 8d1dbd1 into master Jul 16, 2020
@kubermatic-bot kubermatic-bot deleted the dnf-modules-fix branch July 16, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irozzo-1A irozzo-1A irozzo-1A left review comments

@xmudrii xmudrii xmudrii approved these changes

@moadqassem moadqassem Awaiting requested review from moadqassem

Assignees

@moadqassem moadqassem

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kron4eg @kubermatic-bot @moadqassem @irozzo-1A @xmudrii