Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] longhorn-engine integration-test fails after introducing the fix of golangci-lint error #8548

Open
derekbit opened this issue May 11, 2024 · 1 comment
Assignees
Labels
kind/bug kind/regression Regression which has worked before priority/0 Must be fixed in this release (managed by PO) require/backport Require backport. Only used when the specific versions to backport have not been definied. require/qa-review-coverage Require QA to review coverage
Milestone

Comments

@derekbit
Copy link
Member

derekbit commented May 11, 2024

Describe the bug

fix: golangci-lint error, such as longhorn/sparse-tools@813ac9e, longhorn/go-iscsi-helper#89 and so on, introduce a default case in the select statement to address golangci-lint errors:

	mergedErrc := mergeErrorChannels(ctx, errorChannels...)
	select {
	case err = <-mergedErrc:
		break
	default: <------------
	}

However, including the default case causes the select statement to exit immediately if there's no message in mergedErrc, instead of blocking until a message is received.

To Reproduce

Expected behavior

Support bundle for troubleshooting

Environment

  • Longhorn version:
  • Impacted volume (PV):
  • Installation method (e.g. Rancher Catalog App/Helm/Kubectl):
  • Kubernetes distro (e.g. RKE/K3s/EKS/OpenShift) and version:
    • Number of control plane nodes in the cluster:
    • Number of worker nodes in the cluster:
  • Node config
    • OS type and version:
    • Kernel version:
    • CPU per node:
    • Memory per node:
    • Disk type (e.g. SSD/NVMe/HDD):
    • Network bandwidth between the nodes (Gbps):
  • Underlying Infrastructure (e.g. on AWS/GCE, EKS/GKE, VMWare/KVM, Baremetal):
  • Number of Longhorn volumes in the cluster:

Additional context

longhorn/longhorn-engine#1098

@derekbit derekbit added kind/bug require/qa-review-coverage Require QA to review coverage require/backport Require backport. Only used when the specific versions to backport have not been definied. kind/regression Regression which has worked before labels May 11, 2024
@derekbit derekbit added this to the v1.7.0 milestone May 11, 2024
@derekbit derekbit added the priority/0 Must be fixed in this release (managed by PO) label May 11, 2024
@derekbit derekbit self-assigned this May 11, 2024
@longhorn-io-github-bot
Copy link

longhorn-io-github-bot commented May 11, 2024

Pre Ready-For-Testing Checklist

  • Where is the reproduce steps/test steps documented?
    The reproduce steps/test steps are at:

git clone longhorn-engine and run make integration-test

  • Does the PR include the explanation for the fix or the feature?

  • Have the backend code been merged (Manager, Engine, Instance Manager, BackupStore etc) (including backport-needed/*)?
    The PR is at

longhorn/sparse-tools#103
longhorn/go-iscsi-helper#89

  • Which areas/issues this PR might have potential impacts on?
    Area: longhorn-engine
    Issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug kind/regression Regression which has worked before priority/0 Must be fixed in this release (managed by PO) require/backport Require backport. Only used when the specific versions to backport have not been definied. require/qa-review-coverage Require QA to review coverage
Projects
None yet
Development

No branches or pull requests

2 participants