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

Upgrade Harness test is broken #7527

Closed
kvalliyurnatt opened this issue Feb 1, 2024 · 4 comments · Fixed by #7528
Closed

Upgrade Harness test is broken #7527

kvalliyurnatt opened this issue Feb 1, 2024 · 4 comments · Fixed by #7528
Labels
>bug Something isn't working >test Related to unit/integration/e2e tests v2.12.0

Comments

@kvalliyurnatt
Copy link
Contributor

Currently the Upgrade Harness test is broken because the test expects the Logstash CR to have a health field in the status, but the Logstash does not have a health field in the status.

type LogstashStatus struct {
	// Version of the stack resource currently running. During version upgrades, multiple versions may run
	// in parallel: this value specifies the lowest version currently running.
	Version string `json:"version,omitempty"`

	// +kubebuilder:validation:Optional
	ExpectedNodes int32 `json:"expectedNodes,omitempty"`
	// +kubebuilder:validation:Optional
	AvailableNodes int32 `json:"availableNodes,omitempty"`

	// ObservedGeneration is the most recent generation observed for this Logstash instance.
	// It corresponds to the metadata generation, which is updated on mutation by the API Server.
	// If the generation observed in status diverges from the generation in metadata, the Logstash
	// controller has not yet processed the changes contained in the Logstash specification.
	ObservedGeneration int64 `json:"observedGeneration,omitempty"`

	// ElasticsearchAssociationStatus is the status of any auto-linking to Elasticsearch clusters.
	ElasticsearchAssociationsStatus commonv1.AssociationStatusMap `json:"elasticsearchAssociationsStatus,omitempty"`

	// MonitoringAssociationStatus is the status of any auto-linking to monitoring Elasticsearch clusters.
	MonitoringAssociationStatus commonv1.AssociationStatusMap `json:"monitoringAssociationStatus,omitempty"`

	Selector string `json:"selector"`
}

@robbavey @kaisecheng is this field something that the Logstash team could add ? else we will have to look at modifying the test specifically for Logstash.

@kvalliyurnatt kvalliyurnatt added >bug Something isn't working >test Related to unit/integration/e2e tests v2.12.0 labels Feb 1, 2024
@kaisecheng
Copy link
Contributor

@kvalliyurnatt Could you link the red CI here or provide the command to reproduce the test? On top of my head, we recently added a restriction to the minimum version of Logstash, 8.12.0, running in ECK

@kvalliyurnatt
Copy link
Contributor Author

kvalliyurnatt commented Feb 1, 2024

@kaisecheng we do not have an automated to test for this, this is command to run the test

cd hack/upgrade-test-harness && go run main.go --from-release=v2100 --to-release=upcoming

Above is a sample command I think the --from-release might have changes now after the recent release. Does 8.12.0 add the necessary health field to the status ?

This is the check that is failing I think

@robbavey
Copy link
Member

robbavey commented Feb 1, 2024

@kvalliyurnatt Was this a new requirement for the test harness?

I've added a PR, which I think should add what we need - #7528

@kvalliyurnatt
Copy link
Contributor Author

Thanks @robbavey . This is not a new requirement but I think we were excluding Logstash from the upgrade tests, but at some point it was added to the upgrade test harness and it started failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working >test Related to unit/integration/e2e tests v2.12.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants