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 CoreOS Container Linux (EOL) #6576

Merged
merged 1 commit into from
Aug 28, 2020
Merged

Remove support for CoreOS Container Linux (EOL) #6576

merged 1 commit into from
Aug 28, 2020

Conversation

bmelbourne
Copy link
Contributor

@bmelbourne bmelbourne commented Aug 22, 2020

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
As of May 26, 2020, CoreOS Container Linux reached its end of life and will no longer receive updates.

https://coreos.com/os/eol/

https://cloud.google.com/compute/docs/eol/coreOS

Fedora CoreOS is the official successor to CoreOS Container Linux.

Fedora CoreOS is a new Fedora Edition built specifically for running containerized workloads securely and at scale.

Which issue(s) this PR fixes:

Special notes for your reviewer:
PR changes tested using the following Flatcar Container Linux by Kinvolk stable, beta, alpha and edge versions.

  • 2512.3.0
  • 2513.3.0
  • 2605.0.0
  • 2466.99.0

Does this PR introduce a user-facing change?:

Remove support for CoreOS Container Linux (EOL)

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 22, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @bmelbourne. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 22, 2020
@k8s-ci-robot k8s-ci-robot requested review from bozzo and holmsten August 22, 2020 13:51
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 22, 2020
@bmelbourne
Copy link
Contributor Author

/assign @Atoms

@floryut
Copy link
Member

floryut commented Aug 27, 2020

@bmelbourne Could you please cleanup your git history, kind of a mess right now 😄

@Miouge1
Copy link
Contributor

Miouge1 commented Aug 27, 2020

/approve

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bmelbourne, Miouge1

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2020
@bmelbourne
Copy link
Contributor Author

bmelbourne commented Aug 27, 2020

@bmelbourne Could you please cleanup your git history, kind of a mess right now 😄

@floryut
Can you provide a recommendation as I've been asked to rebase with the master before the PR will be reviewed and now this appears to be the incorrect approach. I'm getting confused and frustrated by whats been happening recently.

I've generally worked on submitting PRs (within DevOps/Dev teams) which get reviewed and approved, and then merge/conflicts are resolved pror to the PR closure.

I didn't rebase on my first few PRs and that was the incorrect approach then, hence the confusion now.

Would this be the correct approach?

$ git checkout master
$ git fetch upstream
$ git rebase upstream/master
$ git push origin master
$ git checkout set-docker-cgroup-driver-systemd
$ git pull origin master

@floryut
Copy link
Member

floryut commented Aug 27, 2020

@bmelbourne Could you please cleanup your git history, kind of a mess right now 😄

@floryut
Can you provide a recommendation as I've been asked to rebase with the master before the PR will be reviewed and now this appears to be the incorrect approach. I'm getting confused and frustrated by whats been happening recently.

I've generally worked on submitting PRs (within DevOps/Dev teams) which get reviewed and approved, and then merge/conflicts are resolved pror to the PR closure.

I didn't rebase on my first few PRs and that was the incorrect approach then, hence the confusion now.

Would this be the correct approach?

$ git checkout master
$ git fetch upstream
$ git rebase upstream/master
$ git push origin master
$ git checkout set-docker-cgroup-driver-systemd
$ git pull origin master

Well that's kind of correct, but indeed before opening a PR I tried to switch to master, fetch spray, then reset --hard my master branch (well not needed but never sure if I push some commits or not) and then checkout -b from it 😄

If you are on your branch already and you have a conflict, fetch spray then rebase spray/master is enough.

As you did a pull with merge instead of a rebase, commits ID changed to preserve your history and so we see changes that won't be applied and commit that will be ignored, not an issue but not really great.

@bmelbourne
Copy link
Contributor Author

@bmelbourne Could you please cleanup your git history, kind of a mess right now 😄

@floryut
Can you provide a recommendation as I've been asked to rebase with the master before the PR will be reviewed and now this appears to be the incorrect approach. I'm getting confused and frustrated by whats been happening recently.
I've generally worked on submitting PRs (within DevOps/Dev teams) which get reviewed and approved, and then merge/conflicts are resolved pror to the PR closure.
I didn't rebase on my first few PRs and that was the incorrect approach then, hence the confusion now.
Would this be the correct approach?

$ git checkout master
$ git fetch upstream
$ git rebase upstream/master
$ git push origin master
$ git checkout set-docker-cgroup-driver-systemd
$ git pull origin master

Well that's kind of correct, but indeed before opening a PR I tried to switch to master, fetch spray, then reset --hard my master branch (well not needed but never sure if I push some commits or not) and then checkout -b from it 😄

If you are on your branch already and you have a conflict, fetch spray then rebase spray/master is enough.

As you did a pull with merge instead of a rebase, commits ID changed to preserve your history and so we see changes that won't be applied and commit that will be ignored, not an issue but not really great.

@floryut
Just to be clear, this was incorrect...

$ git pull --rebase upstream master

What I should be doing if I need to bring my local branch up to date with the upstream master is...

$ git fetch upstream master
$ git rebase upstream/master
<resolve any merge conflicts if necessary>
$ git push

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2020
@bmelbourne
Copy link
Contributor Author

bmelbourne commented Aug 27, 2020

@bmelbourne Could you please cleanup your git history, kind of a mess right now 😄

@floryut
Can you provide a recommendation as I've been asked to rebase with the master before the PR will be reviewed and now this appears to be the incorrect approach. I'm getting confused and frustrated by whats been happening recently.
I've generally worked on submitting PRs (within DevOps/Dev teams) which get reviewed and approved, and then merge/conflicts are resolved pror to the PR closure.
I didn't rebase on my first few PRs and that was the incorrect approach then, hence the confusion now.
Would this be the correct approach?

$ git checkout master
$ git fetch upstream
$ git rebase upstream/master
$ git push origin master
$ git checkout set-docker-cgroup-driver-systemd
$ git pull origin master

Well that's kind of correct, but indeed before opening a PR I tried to switch to master, fetch spray, then reset --hard my master branch (well not needed but never sure if I push some commits or not) and then checkout -b from it 😄
If you are on your branch already and you have a conflict, fetch spray then rebase spray/master is enough.
As you did a pull with merge instead of a rebase, commits ID changed to preserve your history and so we see changes that won't be applied and commit that will be ignored, not an issue but not really great.

@floryut
Just to be clear, this was incorrect...

$ git pull --rebase upstream master

What I should be doing if I need to bring my local branch up to date with the upstream master is...

$ git fetch upstream master
$ git rebase upstream/master
<resolve any merge conflicts if necessary>
$ git push

@floryut
I forgot to ask...what approach would you recommend to clean-up the git history?

In my local branch associated with the PR...

$ git rebase -i

and then squash all commits except for the earliest commit e.g. a1855ec which is the first one I originally created with the PR?

@floryut
Copy link
Member

floryut commented Aug 27, 2020

In this case ? Yes what I would do, hard reset the branch with master from spray, cherry pick then squash commits 👍

@bmelbourne
Copy link
Contributor Author

bmelbourne commented Aug 27, 2020

In this case ? Yes what I would do, hard reset the branch with master from spray, cherry pick then squash commits 👍

Ok I've done the following...

$ git fetch origin master
$ git reset --hard origin/HEAD
<overwrite with my changes only>
$ git commit -am "..."
$ git push --force

And the git history of my 4 outstanding PRs are looking much better now.

Can you take a quick look?

@floryut
Copy link
Member

floryut commented Aug 27, 2020

In this case ? Yes what I would do, hard reset the branch with master from spray, cherry pick then squash commits 👍

Ok I've done the following...

$ git fetch origin master
$ git reset --hard origin/HEAD
<overwrite with my changes only>
$ git commit -am "..."
$ git push --force

And the git history of my 4 outstanding PRs are looking much better now.

Can you take a quick look?

That's way better :) thanks man

@floryut
Copy link
Member

floryut commented Aug 28, 2020

Good job with this one, just to be sure

      {%- elif ansible_os_family in ["Coreos", "Container Linux by CoreOS", "Flatcar", "Flatcar Container Linux by Kinvolk"] -%}
      {%- elif ansible_os_family in ["Flatcar Container Linux by Kinvolk"] -%}

ansible_is_family doesn't need "Flatcar" ? only "Flatcar Container Linux by Kinvolk" is returned ? (for all versions/use case etc..)

@bmelbourne
Copy link
Contributor Author

Good job with this one, just to be sure

      {%- elif ansible_os_family in ["Coreos", "Container Linux by CoreOS", "Flatcar", "Flatcar Container Linux by Kinvolk"] -%}
      {%- elif ansible_os_family in ["Flatcar Container Linux by Kinvolk"] -%}

ansible_is_family doesn't need "Flatcar" ? only "Flatcar Container Linux by Kinvolk" is returned ? (for all versions/use case etc..)

@floryut
Correct, I tested the latest stable, beta, alpha and edge versions of Flatcar virtualbox boxes.

There might be older versions where ansible_os_family returns a different string but it'll be impossible to cover all these combinations. If any users encounter this scenario then they'll have to use an earlier version of kubespray until they can upgrade to the latest version of Flatcar. This would be the same situation for any component which is upgraded or end-of-life.

@floryut
Copy link
Member

floryut commented Aug 28, 2020

Agreed with you no real point to cover every older versions
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 28, 2020
@k8s-ci-robot k8s-ci-robot merged commit 058438a into kubernetes-sigs:master Aug 28, 2020
@bmelbourne bmelbourne deleted the remove-eol-coreos-container-linux branch August 28, 2020 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants