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 missing quotation marks in job examples #35028

Closed

Conversation

ysam12345
Copy link

PR #34266 fix one of the typo in Job example, but there're still many similar typo in other examples/languages.
This PR is to fix all these typos.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 14, 2022
@k8s-ci-robot
Copy link
Contributor

Welcome @ysam12345!

It looks like this is your first PR to kubernetes/website 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/website has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 14, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign tengqm after the PR has been reviewed.
You can assign the PR to them by writing /assign @tengqm in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language language/es Issues or PRs related to Spanish language labels Jul 14, 2022
@k8s-ci-robot k8s-ci-robot requested a review from bells17 July 14, 2022 17:10
@k8s-ci-robot k8s-ci-robot added the language/fr Issues or PRs related to French language label Jul 14, 2022
@k8s-ci-robot k8s-ci-robot requested a review from cdrage July 14, 2022 17:10
@k8s-ci-robot k8s-ci-robot added language/id Issues or PRs related to Indonesian language language/ja Issues or PRs related to Japanese language language/ko Issues or PRs related to Korean language language/uk Issues or PRs related to Ukrainian language language/zh Issues or PRs related to Chinese language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jul 14, 2022
@netlify
Copy link

netlify bot commented Jul 14, 2022

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 5421011
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62d04dd8366a76000932afa3
😎 Deploy Preview https://deploy-preview-35028--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -93,7 +93,7 @@ kind: Job
metadata:
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"batch/v1","kind":"Job","metadata":{"annotations":{},"name":"pi","namespace":"default"},"spec":{"backoffLimit":4,"template":{"spec":{"containers":[{"command":["perl","-Mbignum=bpi","-wle","print bpi(2000)"],"image":"perl","name":"pi"}],"restartPolicy":"Never"}}}}
{"apiVersion":"batch/v1","kind":"Job","metadata":{"annotations":{},"name":"pi","namespace":"default"},"spec":{"backoffLimit":4,"template":{"spec":{"containers":[{"command":["perl","-Mbignum=bpi","-wle","'print bpi(2000)'"],"image":"perl","name":"pi"}],"restartPolicy":"Never"}}}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @ysam12345 . Are the single quotes required for the print command?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @kbhawkey . After doing some test, I found the issue is not related to the single quotes.
At first, I ran into an error when trying to run an example from kubernetes document, which will throw the following error:

$ k apply -f job-with-timeout.yaml
job.batch/pi-with-timeout created

$ k get pod
NAME                    READY   STATUS   RESTARTS   AGE
pi-with-timeout-gj68h   0/1     Error    0          26s
pi-with-timeout-lzfhv   0/1     Error    0          15s

$ k logs pi-with-timeout-gj68h
Can't use an undefined value as an ARRAY reference at /usr/local/lib/perl5/5.36.0/Math/BigInt/Calc.pm line 1049.

And I noticed the first example in Job document can successfully complete:

apiVersion: batch/v1
kind: Job
metadata:
  name: pi
spec:
  template:
    spec:
      containers:
      - name: pi
        image: perl:5.34
        command: ["perl",  "-Mbignum=bpi", "-wle", "'print bpi(2000)'"]
      restartPolicy: Never
  backoffLimit: 4

This Job example has those single quotes, which was added by PR #34266. However, I didn't notice although the pod can complete, the perl execution result still had error:

$ kubectl apply -f https://kubernetes.io/examples/controllers/job.yaml
job.batch/pi created

$ k get pod
NAME       READY   STATUS      RESTARTS   AGE
pi-wv7xh   0/1     Completed   0          6s

$ k logs pi-wv7xh
Useless use of a constant ("print bpi(2000)") in void context at -e line 1.

So adding single quotes didn't fix the error "Can't use an undefined value as an ARRAY reference at /usr/local/lib/perl5/5.36.0/Math/BigInt/Calc.pm line 1049.".
After some test, I found this error only happen when using latest perl image, which is perl:5.36 at this time. Using perl:5.34 won't have this issue:

apiVersion: batch/v1
kind: Job
metadata:
  name: pi
spec:
  template:
    spec:
      containers:
      - name: pi
        image: perl:5.34
        command: ["perl",  "-Mbignum=bpi", "-wle", "print bpi(2000)"]
      restartPolicy: Never
  backoffLimit: 4

Result:

$ k apply -f job-perl-5.34-without-single-quotes.yaml
job.batch/pi created

$ k get pod
NAME       READY   STATUS      RESTARTS   AGE
pi-zxpbd   0/1     Completed   0          15s

$ k logs pi-zxpbd
3.1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679821480865132823066470938446095505822317253594081284811174502841027019385211055596446229489549303819644288109756659334461284756482337867831652712019091456485669234603486104543266482133936072602491412737245870066063155881748815209209628292540917153643678925903600113305305488204665213841469519415116094330572703657595919530921861173819326117931051185480744623799627495673518857527248912279381830119491298336733624406566430860213949463952247371907021798609437027705392171762931767523846748184676694051320005681271452635608277857713427577896091736371787214684409012249534301465495853710507922796892589235420199561121290219608640344181598136297747713099605187072113499999983729780499510597317328160963185950244594553469083026425223082533446850352619311881710100031378387528865875332083814206171776691473035982534904287554687311595628638823537875937519577818577805321712268066130019278766111959092164201989380952572010654858632788659361533818279682303019520353018529689957736225994138912497217752834791315155748572424541506959508295331168617278558890750983817546374649393192550604009277016711390098488240128583616035637076601047101819429555961989467678374494482553797747268471040475346462080466842590694912933136770289891521047521620569660240580381501935112533824300355876402474964732639141992726042699227967823547816360093417216412199245863150302861829745557067498385054945885869269956909272107975093029553211653449872027559602364806654991198818347977535663698074265425278625518184175746728909777727938000816470600161452491921732172147723501414419735685481613611573525521334757418494684385233239073941433345477624168625189835694855620992192221842725502542568876717904946016534668049886272327917860857843838279679766814541009538837863609506800642251252051173929848960841284886269456042419652850222106611863067442786220391949450471237137869609563643719172874677646575739624138908658326459958133904780275901

So instead of adding those single quotes, I think I should remove the single quotes added by PR #34266 and modify those Job examples to use perl:5.34 to avoid the "Can't use an undefined value as an ARRAY reference at /usr/local/lib/perl5/5.36.0/Math/BigInt/Calc.pm line 1049." error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @ysam12345 . When creating a pull request, we recommend that you only include changes in one language.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @kbhawkey . I'll modify the commit to only contain changes in en and update this PR. Thanks.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2022
@k8s-ci-robot
Copy link
Contributor

@ysam12345: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Sea-n
Copy link
Member

Sea-n commented Jul 27, 2022

Hey @ysam12345,

It's been a while since last update, would you like to address issues in the previous comments so that we can move forward?

@ysam12345
Copy link
Author

Hi @Sea-n, sorry that I forgot to update this PR. I noticed this issue has been fixed by PR #34414 at this time. So I will close this PR and create PRs for other languages.

@ysam12345 ysam12345 closed this Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language language/es Issues or PRs related to Spanish language language/fr Issues or PRs related to French language language/id Issues or PRs related to Indonesian language language/ja Issues or PRs related to Japanese language language/ko Issues or PRs related to Korean language language/uk Issues or PRs related to Ukrainian language language/zh Issues or PRs related to Chinese language needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants