Adding DeadMansSnitchIntegration CRD to deadmanssnitch operator #70

R-Lawton · 2020-07-28T16:11:35Z

This PR is for the following changes:

Adding a DeadMansSnitchIntegration CRD that replaces the clusterDeployment controller which handled the creation of just one DMS secret and syncset. The new DMSI allows more then one DeadMans snitch and the matching secret and syncset to be created per cluster.
Watches

Create/Update/Delete of a DeadMansSnitchIntegration will trigger a reconcile request for that DeadMansSnitchIntegration.
Create/Update/Delete of a ClusterDeployment will trigger a reconcile request for each DeadMansSnitchIntegration that matches that ClusterDeployment (using the spec.clusterDeploymentSelector label selector).
Create/Update/Delete of a Secret or SyncSet owned by a ClusterDeployment will trigger a reconcile request for each DeadMansSnitchIntegration that matches that ClusterDeployment

functionality:

Create multiple snitches,secrets and syncset per matching cluster deployment(that have the right matching labels)
Delete snitches from deadmanssnitch.com secrets and syncset by either deleting the clusterdeployment of the snitches etc you want deleted or by deleting the DMSI CR of the snitches etc you wanted deleted

app-sre-bot · 2020-07-28T16:11:37Z

Can one of the admins verify this patch?

openshift-ci-robot · 2020-07-28T16:11:51Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

jewzaam

As noted in a comment, need to 's/deadmansnitch/deadmanssnitch/g'.

PARTIAL REVIEW! I haven't gotten to the controller or mapper and their tests.

deploy/role.yaml

pkg/apis/deadmansnitch/v1alpha1/deadmanssnitchintegration_types.go

grdryn

I've gone through this to try to pick up on anything I can before @jewzaam gets to it again. I've left some comments inline...I hope I'm not being too pernickety! :)

pkg/utils/utils.go

pkg/controller/deadmanssnitchintegration/mappers.go

deploy/crds/deadmanssnitch.managed.openshift.io_deadmanssnitchintegrations_crd.yaml

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

jewzaam

Took a pass through all but the controller tests.

deploy/crds/deadmanssnitch.managed.openshift.io_deadmanssnitchintegrations_crd.yaml

deploy/role.yaml

pkg/controller/deadmanssnitchintegration/mappers.go

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

grdryn · 2020-08-19T17:03:28Z

/lint

openshift-ci-robot

@grdryn: 0 warnings.

In response to this:

/lint

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.

grdryn · 2020-08-24T10:19:26Z

/retest

hack/generate.sh

Makefile

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

pkg/controller/deadmanssnitchintegration/mappers.go

pkg/utils/utils.go

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller_test.go

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

jewzaam · 2020-09-10T16:20:50Z

We need a test for when SnitchNamePostFix is not set as well, forgot to mention. The snitch name should be just the cluster deployment name +base domain.

jewzaam · 2020-09-17T15:13:54Z

looks good!
/approve

holding so we can get the initial osd dsmi added (R-Lawton#1) and internal configs done
/hold

deploy/crds/deadmanssnitch.managed.openshift.io_deadmanssnitchintegrations_crd.yaml

pkg/controller/deadmanssnitchintegration/deadmanssnitchintegration_controller.go

…ner engines

R-Lawton · 2020-09-22T14:52:43Z

/retest

R-Lawton · 2020-09-23T08:30:25Z

/retest

R-Lawton · 2020-09-28T09:07:57Z

/retest

https://issues.redhat.com/browse/OSD-5375 Notes: - this is pending merge and promotion of DMSI itself - this is ahead of having the label set at all but will position us to use it immediately after available: https://issues.redhat.com/browse/SDA-3021

jewzaam · 2020-10-01T21:08:39Z

/lgtm

/hold cancel

openshift-ci-robot · 2020-10-01T21:08:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jewzaam, R-Lawton

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:

~~OWNERS~~ [jewzaam]

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

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 28, 2020

openshift-ci-robot requested review from rafael-azevedo and robotmaxtron July 28, 2020 16:11

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 29, 2020

R-Lawton marked this pull request as ready for review July 29, 2020 10:58

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2020

jewzaam requested changes Jul 31, 2020

View reviewed changes

deploy/role.yaml Outdated Show resolved Hide resolved

deploy/role.yaml Outdated Show resolved Hide resolved

pkg/apis/deadmansnitch/v1alpha1/deadmanssnitchintegration_types.go Outdated Show resolved Hide resolved

R-Lawton requested a review from jewzaam August 10, 2020 13:47

grdryn reviewed Aug 18, 2020

View reviewed changes

jewzaam requested changes Aug 18, 2020

View reviewed changes

openshift-ci-robot reviewed Aug 19, 2020

View reviewed changes

R-Lawton requested a review from grdryn August 24, 2020 09:14

grdryn reviewed Aug 24, 2020

View reviewed changes

R-Lawton force-pushed the intly-9089-DMS-integration-CRD branch 4 times, most recently from 4ea3b5b to 96bf519 Compare August 25, 2020 16:33

jewzaam requested changes Sep 10, 2020

View reviewed changes

R-Lawton requested a review from jewzaam September 17, 2020 12:03

jewzaam mentioned this pull request Sep 17, 2020

Add 'osd' DMSI R-Lawton/deadmanssnitch-operator#1

Merged

openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 17, 2020

mwoodson reviewed Sep 17, 2020

View reviewed changes

R-Lawton and others added 7 commits September 21, 2020 11:17

adding to logs

95e1b33

testing CI getting 404

9b50b2e

Add 'osd' DMSI

d37df93

Fixing metrics and update to have consistent support for other contai…

b6b937e

…ner engines

Update metrics port to not conflict with controller-runtime

efca75a

preventing dms from potentialling DOSing

e39eecc

adding conditional statement to createSnitch

38125d1

R-Lawton force-pushed the intly-9089-DMS-integration-CRD branch from 10dd1e3 to 38125d1 Compare September 21, 2020 10:50

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2020

R-Lawton requested a review from mwoodson September 21, 2020 11:25

R-Lawton and others added 10 commits September 28, 2020 15:50

comments from PR including spelling mistakes

e1edc0d

suggestions from PR

38fe76f

PR suggestions

e76dbc5

combining deletion functions into one

654be06

making postfix optional

68fa7f9

fix import in controller-test

3df3e2b

making tags optional

14779e4

Cleanup SecretName, add test for no Tags

c83ce07

Updated unit tests and moved noalerts logic to hack template

c94a92f

jewzaam approved these changes Oct 1, 2020

View reviewed changes

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

openshift-ci-robot assigned jewzaam Oct 1, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2020

openshift-merge-robot merged commit edf2021 into openshift:master Oct 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding DeadMansSnitchIntegration CRD to deadmanssnitch operator #70

Adding DeadMansSnitchIntegration CRD to deadmanssnitch operator #70

R-Lawton commented Jul 28, 2020 •

edited

app-sre-bot commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

jewzaam left a comment

grdryn left a comment

jewzaam left a comment

grdryn commented Aug 19, 2020

openshift-ci-robot left a comment

grdryn commented Aug 24, 2020

jewzaam commented Sep 10, 2020

jewzaam commented Sep 17, 2020

R-Lawton commented Sep 22, 2020

R-Lawton commented Sep 23, 2020

R-Lawton commented Sep 28, 2020

jewzaam commented Oct 1, 2020

openshift-ci-robot commented Oct 1, 2020

Adding DeadMansSnitchIntegration CRD to deadmanssnitch operator #70

Adding DeadMansSnitchIntegration CRD to deadmanssnitch operator #70

Conversation

R-Lawton commented Jul 28, 2020 • edited

functionality:

app-sre-bot commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

jewzaam left a comment

Choose a reason for hiding this comment

grdryn left a comment

Choose a reason for hiding this comment

jewzaam left a comment

Choose a reason for hiding this comment

grdryn commented Aug 19, 2020

openshift-ci-robot left a comment

Choose a reason for hiding this comment

grdryn commented Aug 24, 2020

jewzaam commented Sep 10, 2020

jewzaam commented Sep 17, 2020

R-Lawton commented Sep 22, 2020

R-Lawton commented Sep 23, 2020

R-Lawton commented Sep 28, 2020

jewzaam commented Oct 1, 2020

openshift-ci-robot commented Oct 1, 2020

R-Lawton commented Jul 28, 2020 •

edited