NE-1269: Replace bindata using embed #905

Miciah · 2023-04-04T22:51:14Z

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets.

.gitignore: Ignore the new, generated pkg/manifests/manifests/ directory.
Makefile: Delete bindata target. Add a manifests target to generate assets under pkg/manifests/ from the top-level manifests/ directory. Make the generate target depend on manifests instead of on update. Make the build, test, test-e2e, test-e2e-list, and verify targets depend on generate. Add a command to delete generated files under the clean target.
go.mod: Drop go-bindata.
go.sum: Regenerate.
hack/update-generated-bindata.sh: Delete file.
assets/: Move from here...
pkg/manifests/assets/: ...to here. New directory.
pkg/manifests/bindata.go: Delete file.
pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value.
(MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file.
pkg/manifests/manifests_test.go (TestManifests): Verify that the CustomResourceDefinitionManifest and NamespaceManifest assets can be read.
tools/tools.go: Delete file. We no longer need to import go-bindata.
vendor/*: Regenerate.

openshift-ci-robot · 2023-04-04T22:51:17Z

@Miciah: This pull request references NE-1269 which is a valid jira issue.

Details

In response to this:

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets.

Makefile: Delete bindata target.

go.mod: Drop go-bindata.

go.sum: Regenerate.

hack/update-generated-bindata.sh: Delete file.

assets/: Move from here...

pkg/manifests/assets/: ...to here. New directory.

pkg/manifests/bindata.go: Delete file.

pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value.
(MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file.

tools/tools.go: Delete file. We no longer need to import go-bindata.

vendor/*: Regenerate.

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.

Miciah · 2023-04-05T06:21:37Z

Lots of bootstrap failures.
/retest

openshift-ci-robot · 2023-04-13T09:26:06Z

@Miciah: This pull request references NE-1269 which is a valid jira issue.

Details

In response to this:

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets.

.gitignore: Ignore the new, generated pkg/manifests/manifests/ directory.

Makefile: Delete bindata target. Add a manifests target to generate assets under pkg/manifests/ from the top-level manifests/ directory.

go.mod: Drop go-bindata.

go.sum: Regenerate.

hack/update-generated-bindata.sh: Delete file.

assets/: Move from here...

pkg/manifests/assets/: ...to here. New directory.

pkg/manifests/bindata.go: Delete file.

pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value.
(MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file.

pkg/manifests/manifests_test.go (TestManifests): Verify that the CustomResourceDefinitionManifest and NamespaceManifest assets can be read.

tools/tools.go: Delete file. We no longer need to import go-bindata.

vendor/*: Regenerate.

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-ci-robot · 2023-04-13T12:21:12Z

@Miciah: This pull request references NE-1269 which is a valid jira issue.

Details

In response to this:

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets.

.gitignore: Ignore the new, generated pkg/manifests/manifests/ directory.

Makefile: Delete bindata target. Add a manifests target to generate assets under pkg/manifests/ from the top-level manifests/ directory. Make the generate target depend on manifests instead of on update. Make the build, test-e2e, and test-e2e-list targets depend on generate. Add a command to delete generated files under the clean target.

go.mod: Drop go-bindata.

go.sum: Regenerate.

hack/update-generated-bindata.sh: Delete file.

assets/: Move from here...

pkg/manifests/assets/: ...to here. New directory.

pkg/manifests/bindata.go: Delete file.

pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value.
(MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file.

pkg/manifests/manifests_test.go (TestManifests): Verify that the CustomResourceDefinitionManifest and NamespaceManifest assets can be read.

tools/tools.go: Delete file. We no longer need to import go-bindata.

vendor/*: Regenerate.

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-ci-robot · 2023-04-13T12:35:10Z

@Miciah: This pull request references NE-1269 which is a valid jira issue.

Details

In response to this:

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets.

.gitignore: Ignore the new, generated pkg/manifests/manifests/ directory.

Makefile: Delete bindata target. Add a manifests target to generate assets under pkg/manifests/ from the top-level manifests/ directory. Make the generate target depend on manifests instead of on update. Make the build, test, test-e2e, test-e2e-list, and verify targets depend on generate. Add a command to delete generated files under the clean target.

go.mod: Drop go-bindata.

go.sum: Regenerate.

hack/update-generated-bindata.sh: Delete file.

assets/: Move from here...

pkg/manifests/assets/: ...to here. New directory.

pkg/manifests/bindata.go: Delete file.

pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value.
(MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file.

pkg/manifests/manifests_test.go (TestManifests): Verify that the CustomResourceDefinitionManifest and NamespaceManifest assets can be read.

tools/tools.go: Delete file. We no longer need to import go-bindata.

vendor/*: Regenerate.

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.

candita · 2023-04-26T15:29:41Z

/assign @frobware
/assign

candita · 2023-04-26T15:29:57Z

/assign @frobware
/assign

candita · 2023-04-26T15:38:16Z

pkg/manifests/manifests.go

+//go:embed assets manifests
+var content embed.FS
+
+// MustAsset returns the bytes for the named assert.


Suggested change

// MustAsset returns the bytes for the named assert.

// MustAsset returns the bytes for the named asset.

Fixed in https://github.com/openshift/cluster-ingress-operator/compare/1a7236f10ce2c47f235967601d9f41685fb1a716..62697d237725ec70f45138087875c3e644b901f1.

candita · 2023-04-26T15:57:10Z

pkg/manifests/manifests.go

+//go:generate cp ../../manifests/00-namespace.yaml manifests/
+//go:generate cp ../../manifests/00-custom-resource-definition.yaml manifests/


A couple of questions. Why are these two files the only ones needed from the manifests/ directory? And why not permanently move them to assets/ instead of creating a temporary directory?

A couple of questions. Why are these two files the only ones needed from the manifests/ directory?

They are the only two files that the ingress-operator render command uses:

cluster-ingress-operator/cmd/ingress-operator/render.go

Lines 42 to 45 in e93984f

files := []string{

manifests.CustomResourceDefinitionManifest,

manifests.NamespaceManifest,

}

#309 added the ingress-operator render command for the installer to use in openshift/installer#2523 for configuring ingresscontrollers at installation time.

And why not permanently move them to assets/ instead of creating a temporary directory?

Then they wouldn't be in the right place for CVO to read them. The manifests still need to be installed by CVO if the cluster admin doesn't configure them with the installer.

Then they wouldn't be in the right place for CVO to read them. The manifests still need to be installed by CVO if the cluster admin doesn't configure them with the installer.

Seems like a worthwhile comment to add so that somebody doesn't move things in the future.

https://github.com/openshift/cluster-ingress-operator/compare/62697d237725ec70f45138087875c3e644b901f1..3dafe96f29556f24a6cfdcd199b669681c83221b adds comments.

Miciah · 2023-04-28T14:57:13Z

https://github.com/openshift/cluster-ingress-operator/compare/3dafe96f29556f24a6cfdcd199b669681c83221b..d6369842170adf3e6927befe9e13a673d95f1c8b removes hack/verify-generated-bindata.sh.

Miciah · 2023-04-28T16:53:41Z

Rebased since #901 merged.

frobware · 2023-05-02T09:25:50Z

/approve

@candita deferring to you for the LGTM.

openshift-ci · 2023-05-02T09:26:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: frobware

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~~ [frobware]

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

frobware · 2023-05-02T09:51:27Z

[sig-arch] events should not repeat pathologically
4 days have passed

/retest

Miciah · 2023-05-02T13:06:32Z

This is a build change that doesn't require docs, PX, or QE.
/label docs-approved
/label px-approved
/label qe-approved

candita · 2023-07-19T21:02:00Z

/lgtm

Miciah · 2023-07-19T21:04:57Z

Rebased for #932.

candita · 2023-07-19T21:06:38Z

/lgtm

Miciah · 2023-07-20T00:26:07Z

e2e-hypershift failed.
/test e2e-hypershift

e2e-aws-operator failed because must-gather failed.
/test e2e-aws-operator

Replace go-bindata using the embed package added in Go 1.16. This change simplifies the build process and reduces the likelihood of merge conflicts when PRs modify assets. This commit resolves NE-1269. https://issues.redhat.com/browse/NE-1269 * .gitignore: Ignore the new, generated pkg/manifests/manifests/ directory. * Makefile: Delete "bindata" target. Add a "manifests" target to generate assets under pkg/manifests/ from the top-level manifests/ directory. Make the "generate" target depend on "manifests" instead of on "update". Make the "build", "test", "test-e2e", "test-e2e-list", and "verify" targets depend on "generate". Add a command to delete generated files under the "clean" target. * go.mod: Drop go-bindata. * go.sum: Regenerate. * hack/update-generated-bindata.sh: * hack/verify-generated-bindata.sh: Delete files. * assets/: Move from here... * pkg/manifests/assets/: ...to here. New directory. * pkg/manifests/bindata.go: Delete file. * pkg/manifests/manifests.go (content): New variable. Store assets as an embed.FS value. (MustAsset, MustAssetString): New functions. Return assets using the new content variable. These function definitions replace ones from the deleted bindata.go file. * pkg/manifests/manifests_test.go (TestManifests): Verify that the CustomResourceDefinitionManifest and NamespaceManifest assets can be read. * tools/tools.go: Delete file. We no longer need to import go-bindata. * vendor/*: Regenerate.

Miciah · 2023-07-20T19:15:09Z

Rebased for #900.

candita · 2023-07-20T19:23:58Z

/lgtm
/remove-hold

openshift-ci-robot · 2023-07-20T20:52:38Z

/retest-required

Remaining retests: 0 against base HEAD b4b5f42 and 2 for PR HEAD 0b761da in total

Miciah · 2023-07-20T22:14:47Z

e2e-aws-operator failed because must-gather failed.
/test e2e-aws-operator

e2e-azure-ovn failed because the installer's post-bootstrap cleanup failed:

level=info msg=Destroying the bootstrap resources...
level=info msg=Time elapsed: 15m41s
Copying kubeconfig to shared dir as kubeconfig-minimal
level=error
level=error msg=Error: retrieving Account "clusterolpog" for Container "ignition": retrieving storage accounts: storage.AccountsClient#List: Failure sending request: StatusCode=0 -- Original Error: context deadline exceeded
level=error
level=error msg=  with azurerm_storage_container.ignition,
level=error msg=  on main.tf line 58, in resource "azurerm_storage_container" "ignition":
level=error msg=  58: resource "azurerm_storage_container" "ignition" {
level=error
level=fatal msg=terraform destroy: failed doing terraform destroy: exit status 1
level=fatal
level=fatal msg=Error: retrieving Account "clusterolpog" for Container "ignition": retrieving storage accounts: storage.AccountsClient#List: Failure sending request: StatusCode=0 -- Original Error: context deadline exceeded
level=fatal
level=fatal msg=  with azurerm_storage_container.ignition,
level=fatal msg=  on main.tf line 58, in resource "azurerm_storage_container" "ignition":
level=fatal msg=  58: resource "azurerm_storage_container" "ignition" {
level=fatal
level=fatal
Installer exit with code 1

/test e2e-azure-ovn

Miciah · 2023-07-20T22:29:38Z

e2e-gcp-operator failed because TestAllowedSourceRanges failed. This might be the same issue as OCPBUGS-13106.
/test e2e-gcp-operator

Miciah · 2023-07-20T23:23:37Z

e2e-azure-operator failed because the network operator reported degraded status:

level=error msg=Cluster operator network Degraded is True with ReconcileError: Panic detected: net/http: abort Handler

This appears to be the same issues as OCPBUGS-15945.
/test e2e-azure-operator

openshift-ci · 2023-07-21T01:24:57Z

@Miciah: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 4, 2023

openshift-ci bot requested review from gcs278 and miheer April 4, 2023 22:52

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch 2 times, most recently from 66d900e to b977341 Compare April 13, 2023 12:20

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch from b977341 to 1a7236f Compare April 13, 2023 12:34

openshift-ci bot assigned candita and frobware Apr 26, 2023

candita reviewed Apr 26, 2023

View reviewed changes

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch 3 times, most recently from 3dafe96 to d636984 Compare April 28, 2023 14:54

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch from d636984 to 5fd4199 Compare April 28, 2023 16:53

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2023

openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels May 2, 2023

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

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2023

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2023

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch from e66f429 to 44b7bdc Compare July 19, 2023 21:04

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

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2023

Miciah force-pushed the NE-1269-replace-bindata-using-embed branch from 44b7bdc to 0b761da Compare July 20, 2023 19:14

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2023

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 20, 2023

openshift-merge-robot merged commit 21d27e5 into openshift:master Jul 21, 2023

Miciah mentioned this pull request Jul 24, 2023

NE-1244: Use permissions instead of the "Contributor" role in Azure CredentialsRequest #929

Merged

This was referenced Jul 26, 2023

**FAILURE** rebase-4.14.0-0.nightly-2023-07-26-001154_amd64-2023-07-26_arm64-2023-07-25 openshift/microshift#2121

Closed

rebase 4.14.0 + fix for router assets' source path openshift/microshift#2122

Merged

Miciah mentioned this pull request Apr 17, 2024

OCPBUGS-32371: Bump openshift/api, and update CRD generation #1045

Merged

lance5890 mentioned this pull request Sep 13, 2025

use safe tls-cipher-suites #1277

Closed

	// MustAsset returns the bytes for the named assert.
	// MustAsset returns the bytes for the named asset.

		//go:generate cp ../../manifests/00-namespace.yaml manifests/
		//go:generate cp ../../manifests/00-custom-resource-definition.yaml manifests/

	files := []string{
	manifests.CustomResourceDefinitionManifest,
	manifests.NamespaceManifest,
	}

NE-1269: Replace bindata using embed #905

NE-1269: Replace bindata using embed #905

Uh oh!

Conversation

Miciah commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 4, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Miciah commented Apr 5, 2023

Uh oh!

openshift-ci-robot commented Apr 13, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 13, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 13, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

candita commented Apr 26, 2023

Uh oh!

candita commented Apr 26, 2023

Uh oh!

candita Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Miciah Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

candita Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Miciah Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

frobware Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

Miciah Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

Miciah commented Apr 28, 2023

Uh oh!

Miciah commented Apr 28, 2023

Uh oh!

frobware commented May 2, 2023

Uh oh!

openshift-ci bot commented May 2, 2023

Uh oh!

frobware commented May 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Miciah commented May 2, 2023

Uh oh!

candita commented Jul 19, 2023

Uh oh!

Miciah commented Jul 19, 2023

Uh oh!

candita commented Jul 19, 2023

Uh oh!

Miciah commented Jul 20, 2023

Uh oh!

Miciah commented Jul 20, 2023

Uh oh!

candita commented Jul 20, 2023

Uh oh!

openshift-ci-robot commented Jul 20, 2023

Uh oh!

Miciah commented Jul 20, 2023

Uh oh!

Miciah commented Jul 20, 2023

Uh oh!

Miciah commented Jul 20, 2023

Uh oh!

openshift-ci bot commented Jul 21, 2023

Uh oh!

Reviewers

Assignees

Miciah commented Apr 4, 2023 •

edited

Loading

openshift-ci-robot commented Apr 4, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 13, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 13, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 13, 2023 •

edited by openshift-ci bot

Loading

frobware commented May 2, 2023 •

edited

Loading