Skip to content

BUILD-284: no longer need to apply csi driver yaml in this operator's make deploy#10

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
gabemontero:nuke-temp-crd-patch
Oct 21, 2021
Merged

BUILD-284: no longer need to apply csi driver yaml in this operator's make deploy#10
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
gabemontero:nuke-temp-crd-patch

Conversation

@gabemontero
Copy link
Copy Markdown
Contributor

/assign @coreydaley

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 15, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabemontero

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:

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

@openshift-ci openshift-ci Bot requested review from coreydaley and otaviof October 15, 2021 18:52
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 15, 2021
@gabemontero
Copy link
Copy Markdown
Contributor Author

gabemontero commented Oct 15, 2021

@coreydaley I'll tag this with BUILD-284 in the PR title if CI passes

@coreydaley
Copy link
Copy Markdown

/hold
/lgtm

@openshift-ci openshift-ci Bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Oct 15, 2021
@gabemontero
Copy link
Copy Markdown
Contributor Author

@coreydaley corrected me, this needs https://github.com/openshift/cluster-storage-operator/pull/198/files#diff-426d734400340596f61c0605a3f6ea1108329ebe7f03d7092995bbb534e38ce7R1-R8 from his CSO PR

but now that we know, we'll leave the hold and associate with build 284

@gabemontero gabemontero changed the title no longer need to apply csi driver yaml in this operator's make deploy BUILD-284: no longer need to apply csi driver yaml in this operator's make deploy Oct 15, 2021
@coreydaley
Copy link
Copy Markdown

/hold cancel
will issue a retest once the new CSO image has been published

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2021
@coreydaley
Copy link
Copy Markdown

/retest

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

14 similar comments
@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@coreydaley
Copy link
Copy Markdown

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2021
@gabemontero
Copy link
Copy Markdown
Contributor Author

Well, for starters @coreydaley , our change here should be done to clear out any confusion, but is effectively irrelevant, since I see

Starting e2e test suite normal with timeout 30m.
# temporary creation of CRD until it lands in openshift/api, openshift/openshift-apiserver, etc.
oc apply -f vendor/github.com/openshift/api/sharedresource/v1alpha1/0000_10_sharedsecret.crd.yaml
customresourcedefinition.apiextensions.k8s.io/sharedsecrets.sharedresource.openshift.io created
oc apply -f vendor/github.com/openshift/api/sharedresource/v1alpha1/0000_10_sharedconfigmap.crd.yaml
customresourcedefinition.apiextensions.k8s.io/sharedconfigmaps.sharedresource.openshift.io created

in the test logs.

I now realize that https://github.com/openshift/csi-driver-shared-resource/blob/master/Makefile#L54-L61 is coming into play.

Next, the problem there in the logs, the daemonset not coming up. Not sure how we landed here, but I suspect there is an issue with the operator creating stuff (perhaps rbac?), but our debug in the csi-driver-shared-resource e2e's does not dump operator logs, nor events, from the same namespace.

I think either

  1. We update the driver e2e's to do that, and retest this PR
  2. We try out https://github.com/openshift/csi-driver-shared-resource-operator/blob/master/Makefile#L53-L60 locally and see if we reproduce

Assuming there is a problem, I'm curious how we landed in this spot, and prior PR e2e's did not catch it.

WDYT?

@gabemontero
Copy link
Copy Markdown
Contributor Author

gabemontero commented Oct 21, 2021

Ignore my prior comment #10 (comment)... no longer creating the CRD in the driver Makefile is needed, but is unrelated to the failure.

@coreydaley and I debugged in chat ... the summary

  • his CSO pr has merged
  • but per @coreydaley 's analysis of the must gather, it is not deploying this operator in the e2e cause it is not on a tech preview cluster yet
  • my story this sprint, https://issues.redhat.com/browse/BUILD-256, is taking that on, along with some follow up

so we have some chicken/egg, staging/coordination, needed here

and one could content that this PR is part of the overlap that exists between BUILD-256 and BUILD-284

I am going to try and get enough of the openshift/release changes for BUILD-256 up so that this PR could at least see our operator here deployed, and we go from there.

@coreydaley
Copy link
Copy Markdown

/retest

@gabemontero
Copy link
Copy Markdown
Contributor Author

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2021
@openshift-merge-robot openshift-merge-robot merged commit 9d2abec into openshift:master Oct 21, 2021
@gabemontero gabemontero deleted the nuke-temp-crd-patch branch October 21, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants