Repository: metal3-io/baremetal-operator
Pull request #1008 (2021-10-25 18:22:30)

Use finalizer for BMC secret.
There is a race condition when deleting namespace using API/CLI that contains BMH and BMC Secrets.
Secrets can be deleting before actual de-provisioning of BMH has started and this leads to broken state with registration error.

By using finalizer that is set for secrets that are OwnerRef`ed this change ensures proper objects deletion order by un-setting secrets finalizer only after deprovisioning has ended, k8s will then be allowed to clean-up remaining BMC secrets.

Signed-off-by: s3rj1k <evasive.gyron@gmail.com>

fmuyassarov: @s3rj1k Can you please share the steps to reproduce it?
I've tried
1. BMO and BMH in the same namespace, then deleted the namespace, but since BMO pod is killed, BMH are stuck (which is expected)
2. BMO and BMH are in different namespaces, then I deleted the BMH namepace, and BMO could delete BMHs successfully and eventually namespace was deleted fine.
s3rj1k: @fmuyassarov 
 - BMO, CAPM3 in namespace A
 - BMH, Machine in namespace B
 - Machines are linked(consumed) to BMH
 - BMHs are all in provisioned state
 - BMHs have at least two secrets, BMC access secret, UserData secret
 - There are at least 3 BMHs/Machines in namespace B
 - Manually deleting namespace B -> stuck 
fmuyassarov: > @fmuyassarov
> 
> * BMO, CAPM3 in namespace A
> * BMH, Machine in namespace B
> * Machines are linked(consumed) to BMH
> * BMHs are all in provisioned state
> * BMHs have at least two secrets, BMC access secret, UserData secret
> * There are at least 3 BMHs/Machines in namespace B
> * Manually deleting namespace B -> stuck

Unfortunately I can't reproduce it on my side with metal3-dev-env with this setup, maybe there is something different if you are running on physical servers.
s3rj1k: @fmuyassarov so what do we do about this? 

Assuming that this change is mostly none-invasive, can we merge this without reproduce from your side?

I have 100% reproducibility and it is a kind of show stopper for me.

Also maybe there is a dependency on K8s version itself? (recent versions had some improvements with GC)
fmuyassarov: > @fmuyassarov so what do we do about this?
> 
> Assuming that this change is mostly none-invasive, can we merge this without reproduce from your side?
> 
> I have 100% reproducibility and it is a kind of show stopper for me.
> 
> Also maybe there is a dependency on K8s version itself? (recent versions had some improvements with GC)

Yes sure, I'm not blocking it, just wanted to understand in which cases would this happen, because I've never faced it before, although I do delete namespace sometimes and assume everything to go smooth. And I'm not seeing any harm of using finilizers. 
/lgtm
fmuyassarov: what's the your kubernetes version @s3rj1k ?
s3rj1k: > what's the your kubernetes version @s3rj1k ?

1.20.8
s3rj1k: @zaneb currently only bmc secret has ownerref, thus only that secret would
have finalizer.

пн, 1 нояб. 2021 г., 18:00 Zane Bitter ***@***.***>:

> ***@***.**** commented on this pull request.
>
> I'm not necessarily opposed to this, but as currently written it will
> leave a lot of Secrets permanently undeletable without manual intervention
> to remove the finalizer:
>
>    - The old BMC secret if the user changes the BMH Spec to point to the
>    new one
>    - The Secrets used for userData/metaData/networkData
>
> If we have to do this then we might as well not use owner references,
> because effectively it's the controller that has to decide when to delete
> these Secrets.
> ------------------------------
>
> In controllers/metal3.io/baremetalhost_controller.go
> <https://github.com/metal3-io/baremetal-operator/pull/1008#discussion_r740323846>
> :
>
> > @@ -534,6 +534,20 @@ func (r *BareMetalHostReconciler) actionDeleting(prov provisioner.Provisioner, i
>  		return actionError{errors.Wrap(err, "failed to remove finalizer")}
>  	}
>
> +	if utils.StringInList(info.bmcCredsSecret.Finalizers, metal3v1alpha1.BareMetalHostFinalizer+"/secret") {
> +		// Remove finalizer from secret to allow deletion
> +		info.bmcCredsSecret.Finalizers = utils.FilterStringFromList(
> +			info.bmcCredsSecret.Finalizers, metal3v1alpha1.BareMetalHostFinalizer+"/secret")
> +
> +		if err := r.Update(context.Background(), info.bmcCredsSecret); err != nil {
> +			return actionError{errors.Wrap(err, fmt.Sprintf("failed to remove finalizer from secret %s in namespace %s",
> +				info.bmcCredsSecret.ObjectMeta.Name, info.bmcCredsSecret.ObjectMeta.Namespace))}
> +		}
> +
> +		info.log.Info("removed secret finalizer",
> +			"remaining", info.bmcCredsSecret.Finalizers)
> +	}
>
> Please move this implementation to the SecretManager as a ReleaseSecret()
> method or something like that.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/metal3-io/baremetal-operator/pull/1008#pullrequestreview-794368204>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACWS34PULH2HDMYHNP2S2QTUJ22R5ANCNFSM5GV5GVGA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>

zaneb: > @zaneb currently only bmc secret has ownerref, thus only that secret would have finalizer.

That doesn't follow, because the code you've added is not conditional on a non-nil ownerRef. It's also untrue, because other Secrets _do_ have owner refs.

[`claimSecret()`](https://github.com/metal3-io/baremetal-operator/pull/1008/files#diff-a883846d77daf3b5a84b2aa01499e7497baed3e1e38ac61357c207c3e5886c48R64) is called (via [`obtainSecretForOwner()`](https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L114)) by both [`AcquireSecret()`](https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L128) (adds owner ref) and [`ObtainSecret()`](https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L138) (no owner ref). Both of these are used in numerous places, not just for the BMC creds:

* https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/baremetalhost_controller.go#L1427 (BMC creds)
* https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/host_config_data.go#L29
* https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/baremetalhost_controller.go#L627
* https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/preprovisioningimage_controller.go#L154
s3rj1k: There is an if on line 72 that checks ownerref precence, way above patch
lines.
On code that adds finalizer.

Also in my env I didn't see ownerref on UserData, but If it was intended to
have that ref it is actually good.

Also I am not opposed to refactor, just pointibg out that in my case the
only ownerref on secret was on bmc secret.


пн, 1 нояб. 2021 г., 18:31 Zane Bitter ***@***.***>:

> @zaneb <https://github.com/zaneb> currently only bmc secret has ownerref,
> thus only that secret would have finalizer.
>
> That doesn't follow, because the code you've added is not conditional on a
> non-nil ownerRef. It's also untrue, because other Secrets *do* have owner
> refs.
>
> claimSecret()
> <https://github.com/metal3-io/baremetal-operator/pull/1008/files#diff-a883846d77daf3b5a84b2aa01499e7497baed3e1e38ac61357c207c3e5886c48R64>
> is called (via obtainSecretForOwner()
> <https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L114>)
> by both AcquireSecret()
> <https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L128>
> (adds owner ref) and ObtainSecret()
> <https://github.com/metal3-io/baremetal-operator/blob/master/pkg/secretutils/secret_manager.go#L138>
> (no owner ref). Both of these are used in numerous places, not just for the
> BMC creds:
>
>    -
>    https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/baremetalhost_controller.go#L1427
>    (BMC creds)
>    -
>    https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/host_config_data.go#L29
>    -
>    https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/baremetalhost_controller.go#L627
>    -
>    https://github.com/metal3-io/baremetal-operator/blob/master/controllers/metal3.io/preprovisioningimage_controller.go#L154
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/metal3-io/baremetal-operator/pull/1008#issuecomment-956386543>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACWS34OT5ZZJGSWHN3TAI5TUJ26FJANCNFSM5GV5GVGA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>

zaneb: Ah yep, you're correct, I misread the indentation.
However, it's still the case that the networkData secret(s) also have an owner ref.
You could also make it conditional on the `ownerIsController` flag being true; that is only the case for the BMC Secret for now. However, this is an extremely non-obvious side-effect that probably warrants its own explicit API flag.
s3rj1k: @zaneb @fmuyassarov PTAL
s3rj1k: /test-integration
s3rj1k: /test-v1b1-integration

s3rj1k: /test-v1b1-integration
s3rj1k: /test-integration
s3rj1k: /test all
metal3-io-bot: @s3rj1k: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456549815302230016) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456549815302230016) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456549815419670528) | true | `/test unit`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456549815302230016) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456549815419670528) | true | `/test unit`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456549815302230016) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456549815419670528) | true | `/test unit`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456549815625191424) | true | `/test generate`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456549815302230016) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456549815419670528) | true | `/test unit`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456549815625191424) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456549815662940160) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
s3rj1k: /retest
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456549815419670528) | true | `/test unit`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456549815625191424) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456549815662940160) | true | `/test golint`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456549815738437632) | true | `/test manifestlint`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456549815625191424) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456549815662940160) | true | `/test golint`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456549815625191424) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456549815662940160) | true | `/test golint`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456550122669215744) | true | `/test manifestlint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456549815662940160) | true | `/test golint`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456550122669215744) | true | `/test manifestlint`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456550122547580928) | true | `/test generate`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456549815319007232) | true | `/test gosec`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456550122669215744) | true | `/test manifestlint`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456550122547580928) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456550122581135360) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456549815352561664) | true | `/test gomod`
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456550122669215744) | true | `/test manifestlint`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456550122547580928) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456550122581135360) | true | `/test golint`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456550122329477120) | true | `/test gosec`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gofmt | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1456550122245591040) | true | `/test gofmt`
unit | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1456550122459500544) | true | `/test unit`
manifestlint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1456550122669215744) | true | `/test manifestlint`
generate | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/generate/1456550122547580928) | true | `/test generate`
golint | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1456550122581135360) | true | `/test golint`
gosec | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gosec/1456550122329477120) | true | `/test gosec`
gomod | 6971170af686e5bb7e4d15d67f780083a7a5c1f4 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1456550122371420160) | true | `/test gomod`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
s3rj1k: /retest
metal3-io-bot: New changes are detected. LGTM label has been removed.
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **zaneb** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["zaneb"]} -->
s3rj1k: Updated OR based on recent comments
@fmuyassarov @zaneb @andfasano PTAL
s3rj1k: /test-v1b1-integration
metal3-io-bot: @s3rj1k: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1457832470207533056) | true | `/test unit`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1457832470207533056) | true | `/test unit`
golint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1457832470182367232) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1457832470207533056) | true | `/test unit`
golint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1457832470182367232) | true | `/test golint`
manifestlint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1457832470215921665) | true | `/test manifestlint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1457832470207533056) | true | `/test unit`
golint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1457832470182367232) | true | `/test golint`
manifestlint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1457832470215921665) | true | `/test manifestlint`
gomod | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1457832470203338752) | true | `/test gomod`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @s3rj1k: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/unit/1457832470207533056) | true | `/test unit`
golint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/golint/1457832470182367232) | true | `/test golint`
manifestlint | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/manifestlint/1457832470215921665) | true | `/test manifestlint`
gomod | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gomod/1457832470203338752) | true | `/test gomod`
gofmt | 8a377492e3a059bc681146d8e55ee3dc10719978 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1008/gofmt/1457832470203338753) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1008). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:s3rj1k).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
s3rj1k: /retest
s3rj1k: /test-v1b1-integration
s3rj1k: /test-v1b1-integration
s3rj1k: /test-v1b1-integration
s3rj1k: /test-v1b1-integration

Patch:
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index bf43c8ee..eb8db0e4 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -526,6 +526,13 @@ func (r *BareMetalHostReconciler) actionDeleting(prov provisioner.Provisioner, i
 	}
 
 	// Remove finalizer to allow deletion
+	secretManager := secretutils.NewSecretManager(info.log, r.Client, r.APIReader)
+
+	err = secretManager.ReleaseSecret(info.bmcCredsSecret)
+	if err != nil {
+		return actionError{err}
+	}
+
 	info.host.Finalizers = utils.FilterStringFromList(
 		info.host.Finalizers, metal3v1alpha1.BareMetalHostFinalizer)
 	info.log.Info("cleanup is complete, removed finalizer",
@@ -624,7 +631,7 @@ func (r *BareMetalHostReconciler) preprovImageAvailable(info *reconcileInfo, ima
 			Namespace: image.ObjectMeta.Namespace,
 		}
 		secretManager := r.secretManager(info.log)
-		networkData, err := secretManager.AcquireSecret(secretKey, info.host, false)
+		networkData, err := secretManager.AcquireSecret(secretKey, info.host, false, false)
 		if err != nil {
 			return false, err
 		}
@@ -1424,7 +1431,7 @@ func (r *BareMetalHostReconciler) getBMCSecretAndSetOwner(request ctrl.Request,
 	reqLogger := r.Log.WithValues("baremetalhost", request.NamespacedName)
 	secretManager := r.secretManager(reqLogger)
 
-	bmcCredsSecret, err := secretManager.AcquireSecret(host.CredentialsKey(), host, true)
+	bmcCredsSecret, err := secretManager.AcquireSecret(host.CredentialsKey(), host, true, true)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
 			return nil, &ResolveBMCSecretRefError{message: fmt.Sprintf("The BMC secret %s does not exist", host.CredentialsKey())}
diff --git a/controllers/metal3.io/preprovisioningimage_controller.go b/controllers/metal3.io/preprovisioningimage_controller.go
index 16622831..6783139f 100644
--- a/controllers/metal3.io/preprovisioningimage_controller.go
+++ b/controllers/metal3.io/preprovisioningimage_controller.go
@@ -151,7 +151,7 @@ func getNetworkDataStatus(secretManager secretutils.SecretManager, img *metal3.P
 		Name:      networkDataSecret,
 		Namespace: img.ObjectMeta.Namespace,
 	}
-	secret, err := secretManager.AcquireSecret(secretKey, img, false)
+	secret, err := secretManager.AcquireSecret(secretKey, img, false, false)
 	if err != nil {
 		return metal3.SecretStatus{}, err
 	}
diff --git a/pkg/secretutils/secret_manager.go b/pkg/secretutils/secret_manager.go
index b7a525ed..05a039ef 100644
--- a/pkg/secretutils/secret_manager.go
+++ b/pkg/secretutils/secret_manager.go
@@ -12,6 +12,13 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+	"github.com/metal3-io/baremetal-operator/pkg/utils"
+)
+
+const (
+	SecretsFinalizer = metal3v1alpha1.BareMetalHostFinalizer + "/secret"
 )
 
 // SecretManager is a type for fetching Secrets whether or not they are in the
@@ -58,7 +65,7 @@ func (sm *SecretManager) findSecret(key types.NamespacedName) (secret *corev1.Se
 // claimSecret ensures that the Secret has a label that will ensure it is
 // present in the cache (and that we can watch for changes), and optionally
 // that it has a particular owner reference.
-func (sm *SecretManager) claimSecret(secret *corev1.Secret, owner client.Object, ownerIsController bool) error {
+func (sm *SecretManager) claimSecret(secret *corev1.Secret, owner client.Object, ownerIsController, addFinalizer bool) error {
 	log := sm.log.WithValues("secret", secret.Name, "secretNamespace", secret.Namespace)
 	needsUpdate := false
 	if !metav1.HasLabel(secret.ObjectMeta, LabelEnvironmentName) {
@@ -98,6 +105,12 @@ func (sm *SecretManager) claimSecret(secret *corev1.Secret, owner client.Object,
 		}
 	}
 
+	if addFinalizer && !utils.StringInList(secret.Finalizers, SecretsFinalizer) {
+		log.Info("setting secret finalizer")
+		secret.Finalizers = append(secret.Finalizers, SecretsFinalizer)
+		needsUpdate = true
+	}
+
 	if needsUpdate {
 		if err := sm.client.Update(context.TODO(), secret); err != nil {
 			return errors.Wrap(err, fmt.Sprintf("failed to update secret %s in namespace %s", secret.ObjectMeta.Name, secret.ObjectMeta.Namespace))
@@ -111,12 +124,12 @@ func (sm *SecretManager) claimSecret(secret *corev1.Secret, owner client.Object,
 // will ensure it is present in the cache (and that we can watch for changes),
 // and optionally that it has a particular owner reference. The owner reference
 // may optionally be a controller reference.
-func (sm *SecretManager) obtainSecretForOwner(key types.NamespacedName, owner client.Object, ownerIsController bool) (*corev1.Secret, error) {
+func (sm *SecretManager) obtainSecretForOwner(key types.NamespacedName, owner client.Object, ownerIsController, addFinalizer bool) (*corev1.Secret, error) {
 	secret, err := sm.findSecret(key)
 	if err != nil {
 		return nil, errors.Wrap(err, fmt.Sprintf("failed to fetch secret %s in namespace %s", key.Name, key.Namespace))
 	}
-	err = sm.claimSecret(secret, owner, ownerIsController)
+	err = sm.claimSecret(secret, owner, ownerIsController, addFinalizer)
 
 	return secret, err
 }
@@ -125,16 +138,37 @@ func (sm *SecretManager) obtainSecretForOwner(key types.NamespacedName, owner cl
 // ensure it is present in the cache (and that we can watch for changes), and
 // that it has a particular owner reference. The owner reference may optionally
 // be a controller reference.
-func (sm *SecretManager) AcquireSecret(key types.NamespacedName, owner client.Object, ownerIsController bool) (*corev1.Secret, error) {
+func (sm *SecretManager) AcquireSecret(key types.NamespacedName, owner client.Object, ownerIsController, addFinalizer bool) (*corev1.Secret, error) {
 	if owner == nil {
 		panic("AcquireSecret called with no owner")
 	}
 
-	return sm.obtainSecretForOwner(key, owner, ownerIsController)
+	return sm.obtainSecretForOwner(key, owner, ownerIsController, addFinalizer)
 }
 
 // ObtainSecret retrieves a Secret and ensures that it has a label that will
 // ensure it is present in the cache (and that we can watch for changes).
 func (sm *SecretManager) ObtainSecret(key types.NamespacedName) (*corev1.Secret, error) {
-	return sm.obtainSecretForOwner(key, nil, false)
+	return sm.obtainSecretForOwner(key, nil, false, false)
+}
+
+// ReleaseSecret removes secrets manager finalizer from specified secret when needed.
+func (sm *SecretManager) ReleaseSecret(secret *corev1.Secret) error {
+	if !utils.StringInList(secret.Finalizers, SecretsFinalizer) {
+		return nil
+	}
+
+	// Remove finalizer from secret to allow deletion
+	secret.Finalizers = utils.FilterStringFromList(
+		secret.Finalizers, SecretsFinalizer)
+
+	if err := sm.client.Update(context.Background(), secret); err != nil {
+		return errors.Wrap(err, fmt.Sprintf("failed to remove finalizer from secret %s in namespace %s",
+			secret.ObjectMeta.Name, secret.ObjectMeta.Namespace))
+	}
+
+	sm.log.Info("removed secret finalizer",
+		"remaining", secret.Finalizers)
+
+	return nil
 }


Test patch:
diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go
index 6aa07159..ab86c653 100644
--- a/controllers/metal3.io/baremetalhost_controller_test.go
+++ b/controllers/metal3.io/baremetalhost_controller_test.go
@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/metal3-io/baremetal-operator/pkg/ironic/bmc"
+	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
 
 	"github.com/stretchr/testify/assert"
 
@@ -442,11 +443,15 @@ func TestAddFinalizers(t *testing.T) {
 
 	tryReconcile(t, r, host,
 		func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
-			t.Logf("finalizers: %v", host.ObjectMeta.Finalizers)
-			if utils.StringInList(host.ObjectMeta.Finalizers, metal3v1alpha1.BareMetalHostFinalizer) {
-				return true
+			t.Logf("host finalizers: %v", host.ObjectMeta.Finalizers)
+			if !utils.StringInList(host.ObjectMeta.Finalizers, metal3v1alpha1.BareMetalHostFinalizer) {
+				return false
 			}
-			return false
+
+			hostSecret := getHostSecret(t, r, host)
+			t.Logf("BMC secret finalizers: %v", hostSecret.ObjectMeta.Finalizers)
+
+			return utils.StringInList(hostSecret.ObjectMeta.Finalizers, secretutils.SecretsFinalizer)
 		},
 	)
 }
@@ -1376,7 +1381,7 @@ func TestDeleteHost(t *testing.T) {
 
 			tryReconcile(t, r, host,
 				func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
-					return fix.Deleted && host == nil
+					return fix.Deleted && host == nil && !utils.StringInList(badSecret.Finalizers, secretutils.SecretsFinalizer)
 				},
 			)
 		})