Skip to content

Allow disable automated cleaning#784

Merged
metal3-io-bot merged 3 commits intometal3-io:masterfrom
Nordix:feature-disk-cleaning/feruz
Apr 1, 2021
Merged

Allow disable automated cleaning#784
metal3-io-bot merged 3 commits intometal3-io:masterfrom
Nordix:feature-disk-cleaning/feruz

Conversation

@fmuyassarov
Copy link
Copy Markdown
Member

@fmuyassarov fmuyassarov commented Feb 2, 2021 •
edited
Loading

Disabling disk cleaning implementation.
Fixes: #626
Related changes in CAPM3: metal3-io/cluster-api-provider-metal3#156

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 2, 2021
@fmuyassarov fmuyassarov mentioned this pull request Feb 10, 2021
Merged
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch 2 times, most recently from b36eaba to 44c2128 Compare February 17, 2021 12:28
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 25, 2021
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from d612e07 to 7d64e51 Compare February 25, 2021 12:47
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 25, 2021
@fmuyassarov fmuyassarov marked this pull request as ready for review February 25, 2021 12:53
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 25, 2021
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Feb 25, 2021

/cc @hardys
/cc @dhellmann
/cc @maelk

@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Feb 25, 2021

/cc @dtantsur

hardys
hardys reviewed Mar 1, 2021
View reviewed changes
Comment thread config/crd/bases/metal3.io_baremetalhosts.yaml Outdated
hardys
hardys reviewed Mar 1, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
hardys
hardys reviewed Mar 1, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
kashifest
kashifest requested changes Mar 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@kashifest kashifest left a comment

Choose a reason for hiding this comment

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

Looks Good. Minor comments in line. +1 for the idea of moving the code peace into a function

Comment thread pkg/provisioner/ironic/ironic.go Outdated
Comment thread pkg/provisioner/ironic/ironic.go Outdated
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch 2 times, most recently from 30d6017 to 553b64c Compare March 4, 2021 11:46
@fmuyassarov fmuyassarov mentioned this pull request Mar 4, 2021
Merged
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 4, 2021

Hi @dhellmann @hardys @kashifest
Thanks for the reviews once more. I have updated the code as requested and opened a PR #166 to update proposal in metal3-docs. PTAL.

@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from 553b64c to 633a2c7 Compare March 4, 2021 17:03
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 4, 2021

@dhellmann Please have look, updated as per the latest changes in the proposal.

dhellmann
dhellmann reviewed Mar 4, 2021
View reviewed changes
Comment thread apis/metal3.io/v1alpha1/baremetalhost_types.go Outdated
Comment thread pkg/provisioner/ironic/ironic.go Outdated
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from 633a2c7 to 9ab8af0 Compare March 4, 2021 23:37
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from 131afdd to ecdc9e6 Compare March 30, 2021 14:43
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 30, 2021
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 30, 2021

/test-integration

@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 30, 2021

@zaneb @hardys I moved update to ValidateManagementAccess(). PTAL. Thank you!

hardys
hardys reviewed Mar 30, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
hardys
hardys reviewed Mar 30, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
zaneb
zaneb reviewed Mar 30, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
Comment thread pkg/provisioner/ironic/ironic.go Outdated
@zaneb zaneb mentioned this pull request Mar 30, 2021
Closed
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from ecdc9e6 to dc5e24b Compare March 30, 2021 18:36
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch 3 times, most recently from 8401bc6 to 933be57 Compare March 31, 2021 11:37
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 31, 2021
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch 2 times, most recently from 6287b6b to b6e8626 Compare March 31, 2021 11:46
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 31, 2021

@zaneb @hardys do you think it is better now?

hardys
hardys reviewed Mar 31, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
hardys
hardys reviewed Mar 31, 2021
View reviewed changes
Comment thread pkg/provisioner/ironic/ironic.go Outdated
@fmuyassarov fmuyassarov force-pushed the feature-disk-cleaning/feruz branch from b6e8626 to b2e3824 Compare March 31, 2021 16:19
@hardys
Copy link
Copy Markdown
Member

hardys commented Mar 31, 2021

This version lgtm other than resolving the failing tests, thanks @fmuyassarov!

@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 31, 2021

/test-integration

@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Mar 31, 2021

Thanks everyone for reviews. I think all the comments now addressed!.

@zaneb
Copy link
Copy Markdown
Member

zaneb commented Apr 1, 2021

/lgtm

@hardys hardys mentioned this pull request Apr 1, 2021
Merged
@fmuyassarov fmuyassarov mentioned this pull request Jun 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhellmann dhellmann Awaiting requested review from dhellmann

@dtantsur dtantsur Awaiting requested review from dtantsur

@kashifest kashifest Awaiting requested review from kashifest

@zaneb zaneb Awaiting requested review from zaneb

@hardys hardys Awaiting requested review from hardys

4 more reviewers

@flaper87 flaper87 flaper87 left review comments

@maelk maelk maelk left review comments

@furkatgofurov7 furkatgofurov7 furkatgofurov7 left review comments

@andfasano andfasano andfasano requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. 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.

Configurable disk cleaning options