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

fix(dashboard): Update pod status logic to support native sidecars. Fixes #3366 #3639

Merged
merged 5 commits into from
Jul 31, 2024

Conversation

martynd
Copy link
Contributor

@martynd martynd commented Jun 15, 2024

Closes #3366
Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

Signed-off-by: Martyn Dale <mdale@verticalscope.com>
Copy link

sonarcloud bot commented Jun 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

codecov bot commented Jun 15, 2024

Codecov Report

Attention: Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 84.34%. Comparing base (3024c2a) to head (3bd5f26).
Report is 67 commits behind head on master.

Files with missing lines Patch % Lines
pkg/kubectl-argo-rollouts/info/pod_info.go 12.50% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3639      +/-   ##
==========================================
+ Coverage   84.23%   84.34%   +0.10%     
==========================================
  Files         154      154              
  Lines       18014    18021       +7     
==========================================
+ Hits        15174    15199      +25     
+ Misses       1996     1989       -7     
+ Partials      844      833      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jun 15, 2024

Go Published Test Results

2 171 tests   2 171 ✅  2m 54s ⏱️
  119 suites      0 💤
    1 files        0 ❌

Results for commit b4c79af.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jun 15, 2024

E2E Tests Published Test Results

  4 files    4 suites   3h 24m 0s ⏱️
111 tests 101 ✅  6 💤 4 ❌
452 runs  420 ✅ 24 💤 8 ❌

For more details on these failures, see this check.

Results for commit b4c79af.

♻️ This comment has been updated with latest results.

@Hronom
Copy link

Hronom commented Jun 17, 2024

Any news to make it not draft?

@martynd
Copy link
Contributor Author

martynd commented Jul 2, 2024

Any news to make it not draft?

I need a change to add tests as its failing the coverage checks. Theres no existing tests for any of this however to just extend which complicates it a little

@Hronom
Copy link

Hronom commented Jul 2, 2024

Nice, can't wait for support for native sidecars. Right now Argo Rollouts some sort of blocker for it

@rsavage-nozominetworks
Copy link

Hi folks, any way to make this a priority? Seeing the same issue here.

@martynd martynd marked this pull request as ready for review July 10, 2024 19:03
@martynd
Copy link
Contributor Author

martynd commented Jul 10, 2024

Ive still not got around to doing the tests but I've marked it as ready anyway in case the maintainers are willing to waive the requirement given the absence of testing for for all the functionality there in any form

@rsavage-nozominetworks
Copy link

Ive still not got around to doing the tests but I've marked it as ready anyway in case the maintainers are willing to waive the requirement given the absence of testing for for all the functionality there in any form

How can we get the maintainers involved? Seems like they are ignoring this issue. I posted a message in the argo-rollouts slack channel yesterday, and zero response.

@tbronchain
Copy link

Facing the issue also.

I manage to still use Argo (rollouts/cd) but the rollout status is not properly reported to ArgoCD, which seems to trigger a "bug" with some replicaset showing as ProgressDeadlineExceeded (#3457).

The Issue seem to be "cosmetics" only (rollouts are properly promoted) but I'm wondering if some extra logic is being used during deployments, it may break.

Big thanks to whoever will look into this :)

@zachaller zachaller added this to the v1.8 milestone Jul 11, 2024
@zachaller zachaller self-assigned this Jul 25, 2024
@zachaller
Copy link
Collaborator

Can you fix the lint issue by running make lint

martynd and others added 2 commits July 25, 2024 15:08
Signed-off-by: Martyn Dale <mdale@verticalscope.com>
@Hronom
Copy link

Hronom commented Jul 25, 2024

Hello, do you have sense when it will be released approximately?

Asking because it becomes a major blocker to enable sidecars nowadays if you have in your stack Argo Rollouts.

Copy link
Contributor

github-actions bot commented Jul 25, 2024

Published E2E Test Results

  4 files    4 suites   3h 30m 58s ⏱️
111 tests  95 ✅  6 💤 10 ❌
460 runs  421 ✅ 24 💤 15 ❌

For more details on these failures, see this check.

Results for commit 3bd5f26.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jul 25, 2024

Published Unit Test Results

2 174 tests   2 174 ✅  2m 54s ⏱️
  119 suites      0 💤
    1 files        0 ❌

Results for commit 3bd5f26.

♻️ This comment has been updated with latest results.

Copy link

sonarcloud bot commented Jul 29, 2024

@zachaller zachaller merged commit 6c873a9 into argoproj:master Jul 31, 2024
24 of 25 checks passed
@Hronom
Copy link

Hronom commented Aug 12, 2024

Hello, @zachaller do you have sense when it will be released?

@zachaller zachaller removed this from the v1.8 milestone Aug 12, 2024
zachaller pushed a commit that referenced this pull request Aug 13, 2024
…ixes #3366 (#3639)

* Update pod status logic to support native sidecars

Signed-off-by: Martyn Dale <mdale@verticalscope.com>

* Fix lint issues

Signed-off-by: Martyn Dale <mdale@verticalscope.com>

---------

Signed-off-by: Martyn Dale <mdale@verticalscope.com>
@zachaller zachaller added the cherry-pick-completed Used once we have cherry picked the PR to all requested releases label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick/release-1.7 cherry-pick-completed Used once we have cherry picked the PR to all requested releases
Projects
None yet
5 participants