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

Upgrade 1.13 to Hugo 0.57.2 #16104

Merged
merged 2 commits into from
Oct 2, 2019

Conversation

zacharysarah
Copy link
Contributor

@zacharysarah zacharysarah commented Aug 27, 2019

Fixes #16066

EDIT: #16066 exhibits behavior characteristic of gohugoio/hugo#5615. To resolve the issue, we need to upgrade Hugo to a version that includes the solution.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Aug 27, 2019
@netlify
Copy link

netlify bot commented Aug 27, 2019

Deploy preview for k8s-v1-13 ready!

Built with commit bba51a0

https://deploy-preview-16104--k8s-v1-13.netlify.com

@zacharysarah
Copy link
Contributor Author

The current commit (6bf1a3b) uses a different variable invocation but produces identical results to the production build.
/shrug

@k8s-ci-robot k8s-ci-robot added the ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯ label Aug 27, 2019
Invoke site param as a string

Invoke site param as text

Remove leading dot

Invoke as a page variable

Tweak the page param invocation

lower case

Update Hugo version to 0.57.2
@zacharysarah zacharysarah changed the title [WIP] Populate null href in deprecation warning Upgrade the site to Hugo 0.57.2 Aug 28, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 28, 2019
@zacharysarah
Copy link
Contributor Author

zacharysarah commented Aug 28, 2019

After poking around at possible solutions, this PR changed course to address similar behavior described in gohugoio/hugo#5615. @BenTheElder Thanks for raising visibility! 🙇

To verify the fix:

  1. Go to https://deploy-preview-16104--k8s-v1-13.netlify.com/docs/home/

  2. Verify that the "latest version" link points to https://kubernetes.io/docs/home/.

  3. Repeat with other pages at https://deploy-preview-16104--k8s-v1-13.netlify.com as needed.

@zacharysarah
Copy link
Contributor Author

/assign @sftim

@sftim
Copy link
Contributor

sftim commented Aug 28, 2019

👀

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@sftim
Copy link
Contributor

sftim commented Aug 28, 2019

I have a suspicion that Hugo is having problems turning <h2> elements into a table of contents, if the text contains an apostrophe character, eg:

## Exceed a Container's memory limit

@zacharysarah
Copy link
Contributor Author

@sftim
Copy link
Contributor

sftim commented Aug 28, 2019

https://github.com/kubernetes/website/blame/abde92139926cfb22cecf06acca207c2b0f85f86/content/en/docs/tasks/configure-pod-container/assign-memory-resource.md#L129 has an apostrophe in a second-level heading.

It seems OK to have a What's next section that renders (in the English localization) with an apostrophe. My hypothesis is that apostrophes in the source markdown (lines starting ##) are a problem.

@zacharysarah
Copy link
Contributor Author

zacharysarah commented Aug 28, 2019

@sftim

It seems OK to have a What's next section that renders (in the English localization) with an apostrophe. My hypothesis is that apostrophes in the source markdown (lines starting ##) are a problem.

That checks out. The "What's Next" line populates from a template:

{{% capture whatsnext %}}

But it scrambles any content that follows.

It also looks like Hugo is trying to populate "error" sections with another template: https://github.com/kubernetes/website/blob/master/layouts/partials/templates/errorthrower.html

UPDATE 1: Escaping the apostrophe in the title doesn't resolve the error.

## Exceed a Container\'s memory limit

UPDATE 2: It's more than punctuation. Check out https://deploy-preview-16104--k8s-v1-13.netlify.com/docs/concepts/overview/kubernetes-api/, which has no punctuation in any of its headings:

https://raw.githubusercontent.com/kubernetes/website/master/content/en/docs/concepts/overview/kubernetes-api.md

I think we're looking at broken template handling between 0.53 and 0.57.2.

@zacharysarah
Copy link
Contributor Author

I've opened a Hugo issue: gohugoio/hugo#6277

In the meantime, there is literally no way this PR should merge.

/close

@k8s-ci-robot
Copy link
Contributor

@zacharysarah: Closed this PR.

In response to this:

I've opened a Hugo issue: gohugoio/hugo#6277

In the meantime, there is literally no way this PR should merge.

/close

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.

@zacharysarah
Copy link
Contributor Author

zacharysarah commented Aug 30, 2019

From gohugoio/hugo#6277 (comment):

I just tested the Kubernetes site with the version in #13680 with the latest Hugo and it works fine. You're right that it doesn't work with the current Kubernetes master, but that must just mean that someone has somehow undone the fixes that I applied.

The commit hash for #13680 is b51bdc8.

Only one file affected by #13680 has received subsequent updates: layouts/case-studies/list.html, updated in #13832.

At https://kubernetes.io/case-studies/ the alt text attribute renders as expected:

<img src=https://d33wubrfki0l68.cloudfront.net/1f25fd3bb7209410ad60613770321deb5959c2e6/e2c98/images/case_studies/story.png alt="Tell your story"></a>

UPDATE: I think we may need to cherry-pick b51bdc8 into the release-1.13 branch.

@sftim
Copy link
Contributor

sftim commented Aug 30, 2019

[I hope] I can also take some time this coming Sunday to look into the issue more.

@BenTheElder
Copy link
Member

BenTheElder commented Aug 30, 2019

trying something re:travis
/close

EDIT: no such luck 😞

@k8s-ci-robot
Copy link
Contributor

@BenTheElder: Closed this PR.

In response to this:

trying something re:travis
/close

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.

@BenTheElder
Copy link
Member

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Aug 30, 2019
@k8s-ci-robot
Copy link
Contributor

@BenTheElder: Reopened this PR.

In response to this:

/reopen

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.

@BenTheElder
Copy link
Member

I think a repo admin can /override continuous-integration/travis-ci/pr to have prow force it green https://prow.k8s.io/command-help#override but of course just clicking merge works

@jimangel
Copy link
Member

Looks like it passed?

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2019
@zacharysarah
Copy link
Contributor Author

@chenopis 👋 Hey there, PR wrangler! Could you please /approve this? Thanks in advance.

@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 2, 2019

hi @zacharysarah . I read through the changes. For my information, this branch built a month ago with Hugo 0.57.2? I am trying to understand the open/close and back/forth of this PR. What changed to make the branch pass?
Also, what about o cherry pick the Travis-killing commit into this PR or admin merge it?

@zacharysarah
Copy link
Contributor Author

zacharysarah commented Oct 2, 2019

@kbhawkey Thanks for the review.

I am trying to understand the open/close and back/forth of this PR. What changed to make the branch pass? Also, what about o cherry pick the Travis-killing commit into this PR or admin merge it?

See 16104#issuecomment-526452509 ☝️ for details on both.

EDIT: I'm pretty sure I cherry-picked the Travis-killing commit and squashed it. In any case, I'm not about to look a gift horse in the mouth. 🐴 👀 😴 🤞

@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 2, 2019

/approve

@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 2, 2019

/lgtm

@jimangel
Copy link
Member

jimangel commented Oct 2, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jimangel, kbhawkey

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

The pull request process is described 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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 2, 2019
@k8s-ci-robot k8s-ci-robot merged commit 921adb4 into kubernetes:release-1.13 Oct 2, 2019
k8s-ci-robot pushed a commit that referenced this pull request Oct 2, 2019
Modify apparmor.md and delete .profile to enable the site to
build with Hugo 0.57.2. Could not cherry pick PR 16531 because
the Korean translation was added in 1.14.
See also PRs #16104 and #16151.

zh/docs/reference/setup-tools/kubeadm/generated/README.md updated to fix
'invalid YAML delimiter' compile error caused by HTML comment.

Signed-off-by: Aimee Ukasick <aimeeu.opensource@gmail.com>
@zacharysarah zacharysarah deleted the release-1.13 branch October 2, 2019 18:22
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.

8 participants