Inspection API implementation by fmuyassarov · Pull Request #607 · metal3-io/baremetal-operator

fmuyassarov · 2020-07-29T10:24:46Z

fmuyassarov · 2020-07-29T10:25:07Z

/hold
Need to add unit tests.

zaneb · 2020-08-07T20:37:15Z

-	if !status.Finished {
+	if !status.Finished || (nodes.ProvisionState(ironicNode.ProvisionState) == nodes.Inspecting || nodes.ProvisionState(ironicNode.ProvisionState) == nodes.InspectWait) {
+		details = nil
 		p.log.Info("inspection in progress", "started_at", status.StartedAt)


As an alternative to the inspect flag... we know the time at which the inspection started (and, from memory, ended if it is complete?), and we also know the time at which we entered the Inspecting state (from Host.Status.OperationHistory.Inspect.Start). So we could assume that if any existing inspection was done before we entered the inspecting state, then we need to start a new inspection.

fmuyassarov · 2020-09-15T08:23:26Z

/hold cancel

fmuyassarov

@zaneb Thanks a lot for the review and sorry for addressing the comments late.

fmuyassarov · 2020-09-15T09:33:34Z

/cc @maelk

dhellmann · 2020-09-24T15:43:09Z

I would like to freeze go code changes for a few days to try to land #650 without having to rebase it, because rebasing will mean redoing the work from scratch.

/hold

dhellmann · 2020-10-01T15:27:22Z

#655 has merged

/hold cancel

fmuyassarov · 2021-03-28T20:14:18Z

/test-integration

fmuyassarov · 2021-03-29T06:29:51Z

@andfasano @zaneb If you think #607 (comment) is also addressed, can we forward and merge the PR?

fmuyassarov · 2021-03-29T19:33:02Z

Seems I need to rebase. Are we ok to merge this @zaneb once I do the rebase?

fmuyassarov · 2021-03-29T23:21:06Z

/test-integration

s3rj1k · 2021-03-30T09:24:01Z

@fmuyassarov There is an interesting comment from @zaneb that could probably be fixed in this PR
#817 (comment)
Please take a look, thanks.

fmuyassarov · 2021-03-30T10:02:57Z

@fmuyassarov There is an interesting comment from @zaneb that could probably be fixed in this PR
#817 (comment)
Please take a look, thanks.

Thanks @s3rj1k. If I understood right, err = nil is something that needs to be fixed. I can definitely fix it here, but I was expecting to get this patch in if all the comments have been addressed already.

fmuyassarov · 2021-03-30T10:04:32Z

@fmuyassarov There is an interesting comment from @zaneb that could probably be fixed in this PR
#817 (comment)
Please take a look, thanks.

Thanks @s3rj1k. If I understood right, err = nil is something that needs to be fixed. I can definitely fix it here, but I was expecting to get this patch in if all the comments have been addressed already.

maybe we could fix it in a follow-up PR or within #817?

s3rj1k · 2021-03-30T10:13:42Z

@fmuyassarov There is an interesting comment from @zaneb that could probably be fixed in this PR
#817 (comment)
Please take a look, thanks.

Thanks @s3rj1k. If I understood right, err = nil is something that needs to be fixed. I can definitely fix it here, but I was expecting to get this patch in if all the comments have been addressed already.

maybe we could fix it in a follow-up PR or within #817?

Yep, we need @zaneb feedback here

fmuyassarov · 2021-03-31T21:33:48Z

@zaneb It would be nice if we get this merged if looks okay. Because, there is always something getting merged in master, and I have to rebase it again and again.

zaneb

/approve

metal3-io-bot · 2021-04-01T00:19:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmuyassarov, zaneb

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [zaneb]

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

zaneb · 2021-04-01T00:20:02Z

/test-integration

This adds annotation based API to re-trigger inspection for Ready BMH.

fmuyassarov · 2021-04-01T08:00:45Z

/test-integration
@hardys @andfasano Please take a look, I think all the comments are addressed. Thank you

kashifest

/lgtm
/test-centos-integration

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/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 29, 2020

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2020

fmuyassarov changed the title ~~WIP: Re-inspection API implementation~~ WIP: Inspection API implementation Jul 29, 2020

zaneb reviewed Aug 7, 2020

View reviewed changes

fmuyassarov force-pushed the inspection-feruz branch from aaee7c4 to df2403c Compare August 14, 2020 12:44

fmuyassarov force-pushed the inspection-feruz branch 5 times, most recently from 3ca7beb to acc6d3b Compare September 15, 2020 07:17

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 15, 2020

fmuyassarov changed the title ~~WIP: Inspection API implementation~~ Inspection API implementation Sep 15, 2020

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 Sep 15, 2020

fmuyassarov force-pushed the inspection-feruz branch from acc6d3b to 73bb3aa Compare September 15, 2020 08:50

fmuyassarov commented Sep 15, 2020

View reviewed changes

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/controller/baremetalhost/baremetalhost_controller.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go

metal3-io-bot requested a review from maelk September 15, 2020 09:33

fmuyassarov requested a review from zaneb September 15, 2020 09:34

zaneb reviewed Sep 15, 2020

View reviewed changes

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2020

fmuyassarov force-pushed the inspection-feruz branch from 73bb3aa to 36bfa98 Compare September 25, 2020 10:07

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 Sep 25, 2020

fmuyassarov force-pushed the inspection-feruz branch from 36bfa98 to f2b9c49 Compare September 28, 2020 08:07

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 1, 2020

fmuyassarov force-pushed the inspection-feruz branch from bd60b04 to 5773431 Compare March 25, 2021 16:29

andfasano reviewed Mar 26, 2021

View reviewed changes

Comment thread controllers/metal3.io/host_state_machine.go Outdated

fmuyassarov force-pushed the inspection-feruz branch from 5773431 to 19ae839 Compare March 28, 2021 18:04

fmuyassarov force-pushed the inspection-feruz branch 2 times, most recently from ae6f7b6 to 890b742 Compare March 29, 2021 23:17

fmuyassarov requested review from andfasano and zaneb March 30, 2021 06:58

s3rj1k mentioned this pull request Mar 30, 2021

Allocate Ironic port when node exists and has no port allocated. #817

Merged

fmuyassarov force-pushed the inspection-feruz branch from 890b742 to 97452b7 Compare March 31, 2021 21:31

zaneb reviewed Apr 1, 2021

View reviewed changes

Comment thread controllers/metal3.io/baremetalhost_controller.go Outdated

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2021

Re-inspection API implementation

a70ed8f

This adds annotation based API to re-trigger inspection for Ready BMH.

fmuyassarov force-pushed the inspection-feruz branch from 97452b7 to a70ed8f Compare April 1, 2021 07:58

kashifest reviewed Apr 1, 2021

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2021

metal3-io-bot merged commit 682bd4f into metal3-io:master Apr 1, 2021

fmuyassarov mentioned this pull request Apr 1, 2021

Update inspection API proposal status metal3-io/metal3-docs#174

Merged

fmuyassarov deleted the inspection-feruz branch May 3, 2021 13:17

Conversation

fmuyassarov commented Jul 29, 2020

Uh oh!

fmuyassarov commented Jul 29, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zaneb Aug 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fmuyassarov commented Sep 15, 2020

Uh oh!

fmuyassarov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fmuyassarov commented Sep 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dhellmann commented Sep 24, 2020

Uh oh!

dhellmann commented Oct 1, 2020

Uh oh!

Uh oh!

fmuyassarov commented Mar 28, 2021

Uh oh!

fmuyassarov commented Mar 29, 2021

Uh oh!

fmuyassarov commented Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmuyassarov commented Mar 29, 2021

Uh oh!

s3rj1k commented Mar 30, 2021

Uh oh!

fmuyassarov commented Mar 30, 2021

Uh oh!

fmuyassarov commented Mar 30, 2021

Uh oh!

s3rj1k commented Mar 30, 2021

Uh oh!

fmuyassarov commented Mar 31, 2021

Uh oh!

zaneb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

metal3-io-bot commented Apr 1, 2021

Uh oh!

zaneb commented Apr 1, 2021

Uh oh!

fmuyassarov commented Apr 1, 2021

Uh oh!

kashifest left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

fmuyassarov commented Mar 29, 2021 •

edited

Loading