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

Script #917

Merged
merged 1 commit into from
Oct 9, 2024
Merged

Script #917

merged 1 commit into from
Oct 9, 2024

Conversation

R-Lawton
Copy link
Contributor

@R-Lawton R-Lawton commented Oct 8, 2024

The quickstart script was failing as a namespace was missing. The namespace no longer gets installed with the preferred documented method of installing istio via sail. The script should finish properly now and the observability piece should install without issues

KUADRANT_REF=script ./hack/quickstart-setup.sh

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.43%. Comparing base (63f1d28) to head (483227e).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #917      +/-   ##
==========================================
- Coverage   81.49%   81.43%   -0.07%     
==========================================
  Files         102      115      +13     
  Lines        7177     7507     +330     
==========================================
+ Hits         5849     6113     +264     
- Misses        898      966      +68     
+ Partials      430      428       -2     
Flag Coverage Δ
bare-k8s-integration 8.65% <ø> (-0.25%) ⬇️
controllers-integration 71.28% <ø> (+5.96%) ⬆️
envoygateway-integration 48.78% <ø> (-1.53%) ⬇️
gatewayapi-integration 13.81% <ø> (-0.60%) ⬇️
istio-integration 51.78% <ø> (-1.74%) ⬇️
unit 30.40% <ø> (+2.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
api/v1beta1 (u) 90.90% <ø> (ø)
api/v1beta2 (u) 86.61% <ø> (ø)
pkg/common (u) 88.13% <ø> (ø)
pkg/istio (u) 71.51% <100.00%> (ø)
pkg/log (u) 94.73% <ø> (ø)
pkg/reconcilers (u) ∅ <ø> (∅)
pkg/rlptools (u) 85.00% <ø> (-0.84%) ⬇️
controllers (i) 82.44% <64.86%> (-0.62%) ⬇️

see 12 files with indirect coverage changes

@R-Lawton R-Lawton force-pushed the script branch 6 times, most recently from e3e3497 to 2e66854 Compare October 8, 2024 11:36
@R-Lawton R-Lawton marked this pull request as ready for review October 8, 2024 11:36
@R-Lawton R-Lawton marked this pull request as draft October 8, 2024 15:41
…o longer created with istio being installed by sail

Signed-off-by: R-Lawton <[email protected]>
@R-Lawton R-Lawton marked this pull request as ready for review October 8, 2024 20:20
@R-Lawton R-Lawton requested a review from maleck13 October 8, 2024 20:23
@R-Lawton
Copy link
Contributor Author

R-Lawton commented Oct 8, 2024

@maleck13 Looking at the different types of observability pieces in didn't make sense to just overlay the namespace as envoy and istio have different ports and selectors etc. Split out what was needed by istio into its own separate kustomise that the quickstart deploys and the same for envoy that standalone observability doc installs. Both also still deploy the obvserve pieces that oringally were there require.

@@ -2,7 +2,7 @@ apiVersion: telemetry.istio.io/v1alpha1
kind: Telemetry
metadata:
name: namespace-metrics
namespace: gateway-system
namespace: istio-system
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Telemetry resource is a istio thing not a envoy resource. Envoy has there own version but that can be post v1

Copy link
Collaborator

@maleck13 maleck13 left a comment

Choose a reason for hiding this comment

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

worked for me.

@maleck13 maleck13 merged commit b8f0bf6 into main Oct 9, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants