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

Enable OperatingSystemManager by default #10415

Merged
merged 15 commits into from Jul 28, 2022
Merged

Enable OperatingSystemManager by default #10415

merged 15 commits into from Jul 28, 2022

Conversation

ahmedwaleedmalik
Copy link
Member

@ahmedwaleedmalik ahmedwaleedmalik commented Jul 20, 2022 •
edited

Signed-off-by: Waleed Malik ahmedwaleedmalik@gmail.com

What does this PR do / Why do we need it:
Operating System Manager is enabled by default and it's responsible for creating and managing the required configurations for worker nodes. Users can disable OSM per cluster basis.

Previously, since it was an experimental feature we had the OperatingSystemManager feature gate for it. But since it's considered GA now, we are going to remove that feature gate.

Does this PR close any issues?:
Fixes #

Special notes for your reviewer:

Documentation:

Does this PR introduce a user-facing change?:

* Operating System Manager is enabled by default and it's responsible for creating and managing the required configurations for worker nodes. 
* [ACTION REQUIRED] For existing clusters, users need to set `enableOperatingSystemManager` to true in the cluster spec to enable OSM. Existing `MachineDeployments` will not be rotated automatically. To use OSM for existing `MachineDeployments`, the user needs to update the `MachineDeployments` manually. An example for a somewhat benign change that could trigger rotation is to update the .spec.templates.metadata.annotations field of a MachineDeployment. This would result in the annotation being added to the machines and the machines would be rotated.

@kubermatic-bot kubermatic-bot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. sig/api Denotes a PR or issue as being assigned to SIG API. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 20, 2022
@ahmedwaleedmalik ahmedwaleedmalik changed the title Enable OperatingSystemManager by default WIP: Enable OperatingSystemManager by default Jul 20, 2022
@kubermatic-bot kubermatic-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2022
@ahmedwaleedmalik
Copy link
Member Author

/retest

@ahmedwaleedmalik
Enable OperatingSystemManager by default
c2e1663 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik ahmedwaleedmalik changed the title WIP: Enable OperatingSystemManager by default Enable OperatingSystemManager by default Jul 20, 2022
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2022
@ahmedwaleedmalik
Copy link
Member Author

/retest

@ahmedwaleedmalik ahmedwaleedmalik self-assigned this Jul 20, 2022
@ahmedwaleedmalik
Copy link
Member Author

/test pre-kubermatic-opa-e2e

@ahmedwaleedmalik
Merge branch 'master' of https://github.com/kubermatic/kubermatic int…
12f7ead 
…o enable-osm-default
@ahmedwaleedmalik
Update MC and OSM
c51e057 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
Update test data
ba2aa12 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
Copy link
Member Author

/hold

I'll remove the hold myself.

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2022
@ahmedwaleedmalik
Copy link
Member Author

/test pre-kubermatic-api-e2e

@ahmedwaleedmalik
Copy link
Member Author

/test pre-kubermatic-legacy-machine-controller-user-data-e2e

@ahmedwaleedmalik ahmedwaleedmalik mentioned this pull request Jul 22, 2022
Closed
xrstf
xrstf approved these changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

@xrstf xrstf left a comment

Choose a reason for hiding this comment

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

/approve

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

LGTM label has been added.

Git tree hash: 573af1beb1d4609e5711faaf18935d2e26a0878f

@ahmedwaleedmalik
Update OSM and MC images
ab8a385 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
Merge branch 'master' of https://github.com/kubermatic/kubermatic int…
28a1a30 
…o enable-osm-default
@ahmedwaleedmalik
Update fixtures
e0d2063 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
Sync with master
aec163a 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
Revamp migration for OSM
e5f2436 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
refactor
51436eb 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik
refactor code
97d87cd 
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
xrstf
xrstf approved these changes Jul 28, 2022
View reviewed changes
Copy link
Contributor

@xrstf xrstf left a comment

Choose a reason for hiding this comment

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

/approve

Yay, my last OSM PR!

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

LGTM label has been added.

Git tree hash: d49691d048cb90185c36d2faeb5371e0642a36a9

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmedwaleedmalik, xrstf

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

@ahmedwaleedmalik
Copy link
Member Author

/retest
/hold cancel

@kubermatic-bot kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

/override pre-kubermatic-mla-e2e

fixed via #10565

@kubermatic-bot
Copy link
Contributor

@xrstf: Overrode contexts on behalf of xrstf: pre-kubermatic-mla-e2e

In response to this:

/override pre-kubermatic-mla-e2e

fixed via #10565

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.

@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

/hold

temporarily

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@xrstf xrstf removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

/hold

temporarily. also i see other PRs that have no problem with creating hetzner clusters, so either their API is still flaking or this PR actually broke hetzner support somehow.

@kubermatic-bot kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@ahmedwaleedmalik
Copy link
Member Author

aec163a

Umm that's highly unlikely since everything worked fine till aec163a

But we never know 🤷‍♂️

@ahmedwaleedmalik
Copy link
Member Author

/test pre-kubermatic-api-e2e

@ahmedwaleedmalik
Copy link
Member Author 

    hetzner_test.go:137: failed to create cluster: [POST /api/v1/projects/{project_id}/dc/{dc}/clusters][403] createClusterForbidden 
--- FAIL: TestCreateUpdateHetznerCluster (20.83s)
    --- FAIL: TestCreateUpdateHetznerCluster/create_cluster_on_Hetzner (20.83s)

🤔

@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

The same Prowjob also, if you just scroll up, did

--- PASS: TestCreateUpdateHetznerCluster (578.17s)
    --- PASS: TestCreateUpdateHetznerCluster/create_cluster_on_Hetzner (578.17s)

So I doubt this is purely Hetzner related.

@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

Seems like this time the tests are passing.

@xrstf
Copy link
Contributor

xrstf commented Jul 28, 2022

/hold cancel
/retest

@kubermatic-bot kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@kubermatic-bot kubermatic-bot merged commit c9dc698 into kubermatic:master Jul 28, 2022
@ahmedwaleedmalik ahmedwaleedmalik mentioned this pull request Aug 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xrstf xrstf xrstf approved these changes

@moadqassem moadqassem Awaiting requested review from moadqassem

Assignees

@xrstf xrstf

@ahmedwaleedmalik ahmedwaleedmalik

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-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/api Denotes a PR or issue as being assigned to SIG API. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ahmedwaleedmalik @kubermatic-bot @xrstf