Skip to content

oss-distro-docs build doesn't test docs for OSS #51678

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

Closed
nik9000 opened this issue Jan 30, 2020 · 10 comments
Closed

oss-distro-docs build doesn't test docs for OSS #51678

nik9000 opened this issue Jan 30, 2020 · 10 comments
Labels
:Delivery/Build Build or test infrastructure Pretty Bloody Important Team:Delivery Meta label for Delivery team

Comments

@nik9000
Copy link
Member

nik9000 commented Jan 30, 2020

@lcawl opened #51570 which I thought should fail the oss-docs tests. It didn't. It turns out that the build runs:

./gradlew --parallel --scan -Dorg.elasticsearch.build.cache.url=https://gradle-enterprise.elastic.co/cache/ -S --parallel --max-workers=16 -p docs -Dtests.distribution=default -Dignore.tests.seed --build-cache check

The important bit here is:

-Dtests.distribution=default

Which is backwards. This should be:

-Dtests.distribution=oss

When I run:

./gradlew -p docs/ check -Dtests.distribution=oss

The node doesn't even come up.

@nik9000 nik9000 added :Delivery/Build Build or test infrastructure Pretty Bloody Important labels Jan 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

@rjernst
Copy link
Member

rjernst commented Jan 30, 2020

Looks like it's a settings problem:

»    ↓ errors and warnings from /Users/rjernst/code/elastic/elasticsearch/docs/build/testclusters/integTest-0/logs/es.stdout.log ↓
» WARN ][o.e.c.s.SettingsModule   ] [node-0]
»  *************************************************************************************
»  Found index level settings on node level configuration.
»
»  Since elasticsearch 5.x index level settings can NOT be set on the nodes
»  configuration like the elasticsearch.yaml, in system properties or command line
»  arguments.In order to upgrade all indices the settings must be updated via the
»  /${index}/_settings API. Unless all settings are dynamic all indices must be closed
»  in order to apply the upgradeIndices created in the future should use index templates
»  to set default values.
»
»  Please ensure all required values are updated on all indices by executing:
»
»  curl -XPUT 'http://localhost:9200/_all/_settings?preserve_existing=true' -d '{
»    "index.lifecycle.history_index_enabled" : "false"
»  }'
»  *************************************************************************************
»

@mark-vieira
Copy link
Contributor

mark-vieira commented Jan 30, 2020

This looks to be the bit it doesn't like. My question is, based on the message above, why doesn't this break with the default distribution as well? Also that looks to be a warning. Is that what is actually stopping the node from starting?

dakrone added a commit to dakrone/elasticsearch that referenced this issue Jan 30, 2020
This adds the required

```
[role="xpack"]
[testenv="basic"]
```

To the top of the SLM documentation

Relates to elastic#51678
@rjernst
Copy link
Member

rjernst commented Jan 30, 2020

The node not starting was fixed in #51711. I agree it seems like the node should have failed with the default distribution as well; I don't have an explanation there.

Aside from SLM docs files needing annotations (#51711), there are 2 failures left to investigate:

Tests with failures:
 - org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT.test {yaml=reference/setup/restart-cluster/line_50}
 - org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT.test {yaml=reference/ml/anomaly-detection/categories/line_131}

dakrone added a commit that referenced this issue Jan 30, 2020
This adds the required

```
[role="xpack"]
[testenv="basic"]
```

To the top of the SLM documentation

Relates to #51678
@lcawl
Copy link
Contributor

lcawl commented Jan 30, 2020

I'll open a PR to add the missing testenv attribute to categories.asciidoc

@lcawl
Copy link
Contributor

lcawl commented Jan 31, 2020

With respect to the restart-cluster failure, I suspect it's because one of the optional steps in https://www.elastic.co/guide/en/elasticsearch/reference/master/restart-cluster.html involves a machine learning API. Is it possible to skip this page from the OSS-only tests without marking the entire page as "role=xpack" (since I think that would be misleading)?

@rjernst
Copy link
Member

rjernst commented Jan 31, 2020

@lcawl I don't think that is possible within a single file. However, could the ML part be split out to a separate file that is included, and the other file has the xpack role?

@mark-vieira
Copy link
Contributor

mark-vieira commented Apr 15, 2020

Sorry I'm coming back to this issue late. I'd like to understand what we are trying to test here. I assume, the purpose of this is to ensure that all the snippets in /docs work against an OSS distribution. Well... that's simply not the case. Running locally we get a number of failures, primarily related to CCR, which I assume is because this is an x-pack only feature.

https://gradle-enterprise.elastic.co/s/knsgu5iwoh3yi/tests/failed

So then my question is this? What's the "error" here? Is it that those docs should actually be under /x-pack/docs or should we be ignoring these tests, or some other misconfiguration? Did the CCR docs simply "sneak" in because we weren't actually properly testing against an OSS distro?

@nik9000
Copy link
Member Author

nik9000 commented Apr 15, 2020 via email

@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
droberts195 added a commit to droberts195/elasticsearch that referenced this issue Oct 19, 2020
The original comment mentioned issue elastic#48583, but issue elastic#48941
is specifically open for this mute.  However, this is
inappropriate, as the underlying reason the test cannot be
unmuted is the same as for all the other tests skipped with the
comment "Kibana sample data": issues elastic#51572, elastic#51576 and elastic#51678.

Closes elastic#48941
droberts195 added a commit that referenced this issue Oct 19, 2020
The original comment mentioned issue #48583, but issue #48941
is specifically open for this mute.  However, this is
inappropriate, as the underlying reason the test cannot be
unmuted is the same as for all the other tests skipped with the
comment "Kibana sample data": issues #51572, #51576 and #51678.

Closes #48941
droberts195 added a commit that referenced this issue Oct 19, 2020
The original comment mentioned issue #48583, but issue #48941
is specifically open for this mute.  However, this is
inappropriate, as the underlying reason the test cannot be
unmuted is the same as for all the other tests skipped with the
comment "Kibana sample data": issues #51572, #51576 and #51678.

Closes #48941
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
@mark-vieira
Copy link
Contributor

OSS is going away so.... 👋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Pretty Bloody Important Team:Delivery Meta label for Delivery team
Projects
None yet
Development

No branches or pull requests

5 participants