Skip to content

Conversation

estroz
Copy link
Member

@estroz estroz commented Aug 4, 2021

Description of the change: exclude ServiceAccounts in the CSV from generated bundle dir.

Motivation for the change: since OLM owns ServiceAccounts specified in a CSV but will create any in a bundle, those already in a CSV should not be also included in a bundle. See #5119 for details.

/kind bug

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 4, 2021
@estroz
Copy link
Member Author

estroz commented Aug 4, 2021

/cc @joelanford

@estroz
Copy link
Member Author

estroz commented Aug 4, 2021

/cherry-pick v1.10.x

@openshift-cherrypick-robot

@estroz: once the present PR merges, I will cherry-pick it on top of v1.10.x in a new PR and assign it to you.

In response to this:

/cherry-pick v1.10.x

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.

@estroz
Copy link
Member Author

estroz commented Aug 4, 2021

/cherry-pick v1.9.x

@openshift-cherrypick-robot

@estroz: once the present PR merges, I will cherry-pick it on top of v1.9.x in a new PR and assign it to you.

In response to this:

/cherry-pick v1.9.x

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.

@estroz
Copy link
Member Author

estroz commented Aug 4, 2021

Because we're releasing v1.8.2 anyway

/cherry-pick v1.8.x

@openshift-cherrypick-robot

@estroz: once the present PR merges, I will cherry-pick it on top of v1.8.x in a new PR and assign it to you.

In response to this:

Because we're releasing v1.8.2 anyway

/cherry-pick v1.8.x

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.

Copy link
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

/lgtm

objs = append(objs, &c.V1beta1CustomResourceDefinitions[i])
}

// All ServiceAccounts passed in should be written.
Copy link
Member

Choose a reason for hiding this comment

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

Does this do anything useful or was it just broken behavior from the beginning?

Copy link
Member Author

Choose a reason for hiding this comment

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

Broken behavior, based on incorrect assumptions.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-cherrypick-robot

@estroz: new pull request created: #5124

In response to this:

/cherry-pick v1.10.x

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.

@openshift-cherrypick-robot

@estroz: #5120 failed to apply on top of branch "v1.9.x":

Applying: fix(generate): exclude ServiceAccounts in the CSV from generated bundle
Using index info to reconstruct a base tree...
M	Makefile
Falling back to patching base and 3-way merge...
Removing testdata/helm/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Removing testdata/go/v3/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Removing testdata/ansible/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix(generate): exclude ServiceAccounts in the CSV from generated bundle
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick v1.9.x

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.

@openshift-cherrypick-robot

@estroz: #5120 failed to apply on top of branch "v1.8.x":

Applying: fix(generate): exclude ServiceAccounts in the CSV from generated bundle
Using index info to reconstruct a base tree...
M	Makefile
Falling back to patching base and 3-way merge...
Removing testdata/helm/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Removing testdata/go/v3/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Removing testdata/ansible/memcached-operator/bundle/manifests/memcached-operator-controller-manager_v1_serviceaccount.yaml
Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix(generate): exclude ServiceAccounts in the CSV from generated bundle
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

Because we're releasing v1.8.2 anyway

/cherry-pick v1.8.x

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.

estroz pushed a commit to estroz/operator-sdk that referenced this pull request Aug 5, 2021
estroz pushed a commit to estroz/operator-sdk that referenced this pull request Aug 5, 2021
estroz pushed a commit that referenced this pull request Aug 5, 2021
…rated bundle (#5124)

This is an automated cherry-pick of #5120

Signed-off-by: Eric Stroczynski <[email protected]>
estroz pushed a commit that referenced this pull request Aug 5, 2021
…ated bundle (#5126)

Cherry-pick of #5120

Signed-off-by: Eric Stroczynski <[email protected]>
estroz pushed a commit that referenced this pull request Aug 5, 2021
…ated bundle (#5127)

Cherry-pick of #5120

Signed-off-by: Eric Stroczynski <[email protected]>
bentito pushed a commit to bentito/operator-sdk that referenced this pull request Aug 24, 2021
twasyl pushed a commit to twasyl/operator-sdk that referenced this pull request Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. 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