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

docs: Fix port definitions in examples #7308

Closed
wants to merge 2 commits into from
Closed

docs: Fix port definitions in examples #7308

wants to merge 2 commits into from

Conversation

hjorth
Copy link

@hjorth hjorth commented Nov 30, 2021

FIx port definition errors in examples.

Copy link
Member

@simster7 simster7 left a comment

Choose a reason for hiding this comment

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

@hjorth
Copy link
Author

hjorth commented Nov 30, 2021

These were recently changed to strings from ints: https://github.com/argoproj/argo-workflows/pull/4831/files#diff-fbd4cc7bade7a2809ed6ed26bb784529a98b8ef86453e9cec02394be45dce8c7R27

Which is the correct one? cc @kennytrytek

I tested all the examples on 3.1.14, GKE 1.20 - they weren't working.

Just checked daemon-nginx on 3.2.4, it fails with:

step group deemed errored due to child daemon-nginx-6qcjp[0].nginx-server error: Pod "daemon-nginx-6qcjp-3819426501" is invalid: spec.containers[1].readinessProbe.httpGet.port: Invalid value: "80": must contain at least one letter or number (a-z, 0-9)

@kennytrytek
Copy link
Contributor

kennytrytek commented Nov 30, 2021

#4817 (comment)

PR is open to revert back to only support string. IntOrString was previously defined strictly as a string in this commit. It was changed in this pull request to generate the invalid "int or string" spec. While technically closer to the intent of the field name, supporting both types cannot be implemented until Swagger v3.1 is used, as discussed above.

This is my best understanding of where things sit, so I think string is correct. Feel free to correct me if I've missed something. If string doesn't work for those fields, then clearly they shouldn't have been changed.

@alexec
Copy link
Contributor

alexec commented Dec 1, 2021

I think this PR is probably correct. I think this is just vanilla values.

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

However, I think this PR does need to fix the DCO and Semantic Pull Request.

@hjorth
Copy link
Author

hjorth commented Dec 1, 2021

However, I think this PR does need to fix the DCO and Semantic Pull Request.

Will check that out.

@alexec alexec changed the title Fix port definitions in examples docs: Fix port definitions in examples Jan 19, 2022
@stale
Copy link

stale bot commented Feb 7, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the problem/stale This has not had a response in some time label Feb 7, 2022
@stale stale bot closed this Feb 12, 2022
@ZPerling
Copy link

ZPerling commented Sep 15, 2022

now my port is defined by a input parameter

spec:
  templates:
    - name: java-service-run
      inputs:
        parameters:
          - name: run-command
          - name: port
            value: 8080
      daemon: true
      retryStrategy:
        limit: "1"
      container:
        workingDir: '/work/'
        image: jdk:8u333-centos7
        command:
        - '{{inputs.parameters.run-command}}'
        readinessProbe:
          tcpSocket:
            port: "{{inputs.parameters.port}}"
          initialDelaySeconds: 60
          periodSeconds: 6
          failureThreshold: 30

same error occurred
image

I try remove '{{inputs.parameters.run-command}}' quotes like {{inputs.parameters.run-command}}, but can not submit , how can I fix this problem

@agilgur5 agilgur5 added the area/docs Incorrect, missing, or mistakes in docs label Jan 17, 2024
@agilgur5
Copy link

This PR was superseded by #9207

@agilgur5 agilgur5 added solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) and removed problem/stale This has not had a response in some time labels Jul 22, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators Jul 23, 2024
@agilgur5
Copy link

agilgur5 commented Aug 3, 2024

now my port is defined by a input parameter

This isn't related to this PR specifically, but is related to one mentioned above: #4831
For anyone that comes here, If you still have this issue, please open a new issue about that.
Also you might be able to workaround it with podSpecPatch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/docs Incorrect, missing, or mistakes in docs solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants