Skip to content

fix: port range exposing and various bugfixes #809

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Sep 5, 2023
Merged

fix: port range exposing and various bugfixes #809

merged 7 commits into from
Sep 5, 2023

Conversation

@m8vago
Copy link
Contributor

@m8vago m8vago commented Sep 4, 2023 •
edited
Loading

  • Rework agent version check
  • Fix an issue in dagent, where the port ranges weren't exposed.
  • Clarify some comments
  • Fix metrics port validation
  • Add http -> https redirection to the main docker-compose

@m8vago m8vago requested a review from a team as a code owner September 4, 2023 13:07
@github-actions github-actions bot added pr:fix source:web The scope of the issue or pull request is web. lang:golang lang:typescript labels Sep 4, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2023 •
edited
Loading

Codecov Report

Patch coverage: 25.00% and project coverage change: -0.03% ⚠️

Comparison is base (da12e4c) 24.13% compared to head (012bc1a) 24.10%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #809      +/-   ##
===========================================
- Coverage    24.13%   24.10%   -0.03%     
===========================================
  Files           62       62              
  Lines         5958     5965       +7     
===========================================
  Hits          1438     1438              
- Misses        4414     4421       +7     
  Partials       106      106
Files Changed Coverage Δ
golang/pkg/builder/container/container_builder.go 42.09% <25.00%> (-0.85%) ⬇️

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

robot9706
robot9706 reviewed Sep 4, 2023
View reviewed changes
@m8vago m8vago force-pushed the fix/port-range-exposing branch from eb75cf1 to dc33cf3 Compare September 4, 2023 16:07
polaroi8d
polaroi8d requested changes Sep 4, 2023
View reviewed changes
Copy link
Collaborator

@polaroi8d polaroi8d left a comment

Choose a reason for hiding this comment

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

I really need some documentation regarding AGENT_SUPPORTED_MINIMUM_VERSION. We need to clarify how we intend to handle it, what it is, why we are using it, and so on. It's my fault, I didn't add this to the previous PR, sorry.

polaroi8d
polaroi8d requested changes Sep 4, 2023
View reviewed changes
Copy link
Collaborator

@polaroi8d polaroi8d left a comment

Choose a reason for hiding this comment

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

Test case: Added a new project with 1 nginx container. Added a portrange from 7000 to 7500 same in internal and external. Added a port expose 7543 same in internal and external. Tried to deploy it and got the following error message: client.js:1 TypeError: Cannot read properties of null (reading 'enabled')

Otherhand console throws a few next-translate error about "images" for example:
[next-translate] "images:" is missing in current namespace configuration. Try adding "" to the namespace "images".

@m8vago
Copy link
Contributor Author

m8vago commented Sep 5, 2023 •
edited
Loading

I really need some documentation regarding AGENT_SUPPORTED_MINIMUM_VERSION. We need to clarify how we >intend to handle it, what it is, why we are using it, and so on. It's my fault, I didn't add this to the previous PR, sorry.

It's not your fault, i removed this const from the last PR and added it here. I'll write some docs on it.

robot9706
robot9706 reviewed Sep 5, 2023
View reviewed changes
@m8vago
Copy link
Contributor Author

m8vago commented Sep 5, 2023

@polaroi8d pls check the DEVELOPMENT.md

polaroi8d
polaroi8d reviewed Sep 5, 2023
View reviewed changes
robot9706
robot9706 approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@robot9706 robot9706 left a comment

Choose a reason for hiding this comment

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

Tested locally, ranges and validation seem to work fine.

@polaroi8d polaroi8d merged commit 5cb9c7a into develop Sep 5, 2023
@polaroi8d polaroi8d deleted the fix/port-range-exposing branch September 5, 2023 13:39
chandhuDev pushed a commit to chandhuDev/dyrectorio that referenced this pull request Nov 7, 2023
@m8vago @chandhuDev
fix: port range exposing and various bugfixes (dyrector-io#809)
38eed43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robot9706 robot9706 robot9706 approved these changes

@polaroi8d polaroi8d polaroi8d approved these changes

Assignees

@m8vago m8vago

Labels

lang:golang lang:typescript pr:fix source:web The scope of the issue or pull request is web.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@m8vago @codecov-commenter @robot9706 @polaroi8d